-
Notifications
You must be signed in to change notification settings - Fork 1
Up and attach #27
Up and attach #27
Changes from 8 commits
7d881c2
cb22228
a617531
0deceb3
ed64c29
03e2d94
7089eea
62175ac
fa7efb4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
PATH | ||
remote: . | ||
specs: | ||
kaiser (0.4.0) | ||
kaiser (0.4.1) | ||
optimist | ||
|
||
GEM | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,21 @@ | |
module Kaiser | ||
# The commandline | ||
class Cli | ||
@options = [] | ||
|
||
class << self | ||
def option(*option) | ||
@options ||= [] | ||
@options << option | ||
end | ||
|
||
def options | ||
@options || [] | ||
end | ||
end | ||
|
||
attr_reader :opts | ||
|
||
def set_config | ||
# This is here for backwards compatibility since it can be used in Kaiserfiles. | ||
# It would be a good idea to deprecate this and make it more abstract. | ||
|
@@ -29,7 +44,7 @@ def define_options(global_opts = []) | |
# the scope to Optimist::Parser. We can still reference variables but we can't | ||
# call instance methods of a Kaiser::Cli class. | ||
u = usage | ||
Optimist.options do | ||
@opts = Optimist.options do | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of making this global, can we just pass it in as a parameter to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought about too at first but the thing is execute calls a lot of private methods that are often shared between commands. We would have to pass the options to every single one. Or I guess you could only pass them to the ones that actually make use of them which could make it more transparent. You could see at a glance which method uses options and which one does not. I started writing this comment as a counter-argument but finished it thinking maybe you're right. lol It would be less magic that way too. |
||
banner u | ||
|
||
global_opts.each { |o| opt *o } | ||
|
@@ -43,7 +58,7 @@ def self.register(name, klass) | |
|
||
def self.run_command(name, global_opts) | ||
cmd = @subcommands[name] | ||
opts = cmd.define_options(global_opts) | ||
opts = cmd.define_options(global_opts + cmd.class.options) | ||
|
||
# The define_options method has stripped all arguments from the cli so now | ||
# all that we're left with in ARGV are the subcommand to be run and possibly | ||
|
@@ -57,7 +72,6 @@ def self.run_command(name, global_opts) | |
# unless they create a Kaiserfile firest. | ||
out = Kaiser::Dotter.new | ||
info_out = Kaiser::AfterDotter.new(dotter: out) | ||
|
||
if opts[:quiet] | ||
out = File.open(File::NULL, 'w') | ||
info_out = File.open(File::NULL, 'w') | ||
|
@@ -216,6 +230,29 @@ def default_db_image | |
db_image_path('.default') | ||
end | ||
|
||
def attach_app | ||
cmd = (ARGV || []).join(' ') | ||
killrm app_container_name | ||
|
||
attach_mounts = Config.kaiserfile.attach_mounts | ||
volumes = attach_mounts.map { |from, to| "-v #{`pwd`.chomp}/#{from}:#{to}" }.join(' ') | ||
|
||
system "docker run -ti | ||
--name #{app_container_name} | ||
--network #{network_name} | ||
--dns #{ip_of_container(Config.config[:shared_names][:dns])} | ||
--dns-search #{http_suffix} | ||
-p #{app_port}:#{app_expose} | ||
-e DEV_APPLICATION_HOST=#{envname}.#{http_suffix} | ||
-e VIRTUAL_HOST=#{envname}.#{http_suffix} | ||
-e VIRTUAL_PORT=#{app_expose} | ||
#{volumes} | ||
#{app_params} | ||
kaiser:#{envname}-#{current_branch} #{cmd}".tr("\n", ' ') | ||
|
||
Config.out.puts 'Cleaning up...' | ||
end | ||
|
||
def start_app | ||
Config.info_out.puts 'Starting up application' | ||
killrm app_container_name | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
# frozen_string_literal: true | ||
|
||
module Kaiser | ||
VERSION = '0.4.0' | ||
VERSION = '0.4.1' | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This bunch of things is a great thing to abstract into a module. If you define this in a module, say a
CliOptions
module, you canextend CliOptions
in the classCli
and it will be easier to see whats going on.At the moment its really easy to confuse
:opts
andoptions
.Also with a
CliOptions
module renaming @options to@cli_options
would make making the link easier.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potentially you can also add it to only commands that require options if you do it this way. So it looks a lot less like magic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I was also worried about opts and options being confusing. Especially since one is a class instance variable and the other is an instance variable. I'll put them in the module. I also like the idea of less magic. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do think it's better to keep it in the superclass because this line over here relies on the
cmd.class.options
method existing and returning an array.I feel like this is more elegant than writing a check to see if the module was extended or not.