From 6ca7fa47da304af46c356bd3582ad03e47bce5ef Mon Sep 17 00:00:00 2001 From: TAGOMORI Satoshi Date: Wed, 25 Dec 2019 14:44:22 +0900 Subject: [PATCH 1/5] fix the horrible indent and whitespaces --- lib/td/command/runner.rb | 400 +++++++++++++++++++-------------------- lib/td/config.rb | 320 +++++++++++++++---------------- 2 files changed, 356 insertions(+), 364 deletions(-) diff --git a/lib/td/command/runner.rb b/lib/td/command/runner.rb index 8ee73611..dc0e078c 100644 --- a/lib/td/command/runner.rb +++ b/lib/td/command/runner.rb @@ -1,41 +1,41 @@ module TreasureData -module Command - -class Runner - def initialize - @config_path = nil - @apikey = nil - @endpoint = nil - @import_endpoint = nil - @prog_name = nil - @insecure = false - end - - attr_accessor :apikey, :endpoint, :import_endpoint, :config_path, :prog_name, :insecure - - def run(argv=ARGV) - require 'td/version' - require 'td/compat_core' - require 'optparse' - - $prog = @prog_name || File.basename($0) - - op = OptionParser.new - op.version = TOOLBELT_VERSION - op.banner = < e - # known exceptions are rendered as simple error messages unless the - # TD_TOOLBELT_DEBUG variable is set or the -v / --verbose option is used. - # List of known exceptions: - # => ParameterConfigurationError - # => BulkImportExecutionError - # => UpUpdateError - # => ImportError - require 'td/client/api' - # => APIError - # => ForbiddenError - # => NotFoundError - # => AuthError - if ![ParameterConfigurationError, BulkImportExecutionError, UpdateError, ImportError, - APIError, ForbiddenError, NotFoundError, AuthError, AlreadyExistsError, WorkflowError].include?(e.class) || - !ENV['TD_TOOLBELT_DEBUG'].nil? || $verbose - show_backtrace "Error #{$!.class}: backtrace:", $!.backtrace - end - if $!.respond_to?(:api_backtrace) && $!.api_backtrace - show_backtrace "Error backtrace from server:", $!.api_backtrace.split("\n") - end + require 'td/command/list' + if defined?(Encoding) + #Encoding.default_internal = 'UTF-8' if Encoding.respond_to?(:default_internal) + Encoding.default_external = 'UTF-8' if Encoding.respond_to?(:default_external) + end - $stdout.print "Error: " - if [ForbiddenError, NotFoundError, AuthError].include?(e.class) - $stdout.print "#{e.class} - " - end - $stdout.puts $!.to_s + method, cmd_req_connectivity = Command::List.get_method(cmd) + unless method + $stderr.puts "'#{cmd}' is not a td command. Run '#{$prog}' to show the list." + Command::List.show_guess(cmd) + return 1 + end - require 'socket' - if e.is_a?(::SocketError) - $stderr.puts < e + # known exceptions are rendered as simple error messages unless the + # TD_TOOLBELT_DEBUG variable is set or the -v / --verbose option is used. + # List of known exceptions: + # => ParameterConfigurationError + # => BulkImportExecutionError + # => UpUpdateError + # => ImportError + require 'td/client/api' + # => APIError + # => ForbiddenError + # => NotFoundError + # => AuthError + if ![ParameterConfigurationError, BulkImportExecutionError, UpdateError, ImportError, + APIError, ForbiddenError, NotFoundError, AuthError, AlreadyExistsError, WorkflowError].include?(e.class) || + !ENV['TD_TOOLBELT_DEBUG'].nil? || $verbose + show_backtrace "Error #{$!.class}: backtrace:", $!.backtrace + end + + if $!.respond_to?(:api_backtrace) && $!.api_backtrace + show_backtrace "Error backtrace from server:", $!.api_backtrace.split("\n") + end + + $stdout.print "Error: " + if [ForbiddenError, NotFoundError, AuthError].include?(e.class) + $stdout.print "#{e.class} - " + end + $stdout.puts $!.to_s + + require 'socket' + if e.is_a?(::SocketError) + $stderr.puts < Date: Wed, 25 Dec 2019 15:22:09 +0900 Subject: [PATCH 2/5] add ConfigFile class and use its instance for configuration file entries --- lib/td/config.rb | 153 ++++++++++++++++++++++++----------------------- 1 file changed, 78 insertions(+), 75 deletions(-) diff --git a/lib/td/config.rb b/lib/td/config.rb index 25dcc220..1fde7e7e 100644 --- a/lib/td/config.rb +++ b/lib/td/config.rb @@ -11,6 +11,11 @@ class ConfigParseError < ConfigError class Config # class variables @@path = ENV['TREASURE_DATA_CONFIG_PATH'] || ENV['TD_CONFIG_PATH'] || File.join(ENV['HOME'], '.td', 'td.conf') + + def self.read(path=Config.path, create=false) + ConfigFile.new(path) + end + @@apikey = ENV['TREASURE_DATA_API_KEY'] || ENV['TD_API_KEY'] @@apikey = nil if @@apikey == "" @@cl_apikey = false # flag to indicate whether an apikey has been provided through the command-line @@ -23,81 +28,6 @@ class Config @@secure = true @@retry_post_requests = false - def initialize - @path = nil - @conf = {} # section.key = val - end - - def self.read(path=Config.path, create=false) - new.read(path) - end - - def [](cate_key) - @conf[cate_key] - end - - def []=(cate_key, val) - @conf[cate_key] = val - end - - def delete(cate_key) - @conf.delete(cate_key) - end - - def read(path=@path) - @path = path - begin - data = File.read(@path) - rescue - e = ConfigNotFoundError.new($!.to_s) - e.set_backtrace($!.backtrace) - raise e - end - - section = "" - - data.each_line {|line| - line.strip! - case line - when /^#/ - next - when /\[(.+)\]/ - section = $~[1] - when /^(\w+)\s*=\s*(.+?)\s*$/ - key = $~[1] - val = $~[2] - @conf["#{section}.#{key}"] = val - else - raise ConfigParseError, "invalid config line '#{line}' at #{@path}" - end - } - - self - end - - def save(path=@path||Config.path) - @path = path - write - end - - private - def write - require 'fileutils' - FileUtils.mkdir_p File.dirname(@path) - File.open(@path, "w") {|f| - @conf.keys.map {|cate_key| - cate_key.split('.', 2) - }.zip(@conf.values).group_by {|(section,key), val| - section - }.each {|section,cate_key_vals| - f.puts "[#{section}]" - cate_key_vals.each {|(section,key), val| - f.puts " #{key} = #{val}" - } - } - } - end - def self.path @@path end @@ -193,5 +123,78 @@ def self.cl_options_string string end + class ConfigFile + def initialize(path=nil) + @path = path + @conf = {} # section.key = val + read if path + end + + def [](cate_key) + @conf[cate_key] + end + + def []=(cate_key, val) + @conf[cate_key] = val + end + + def delete(cate_key) + @conf.delete(cate_key) + end + + def read(path=@path) + @path = path + begin + data = File.read(@path) + rescue + e = ConfigNotFoundError.new($!.to_s) + e.set_backtrace($!.backtrace) + raise e + end + + section = "" + + data.each_line {|line| + line.strip! + case line + when /^#/ + next + when /\[(.+)\]/ + section = $~[1] + when /^(\w+)\s*=\s*(.+?)\s*$/ + key = $~[1] + val = $~[2] + @conf["#{section}.#{key}"] = val + else + raise ConfigParseError, "invalid config line '#{line}' at #{@path}" + end + } + + self + end + + def save(path=@path||Config.path) + @path = path + write + end + + private + def write + require 'fileutils' + FileUtils.mkdir_p File.dirname(@path) + File.open(@path, "w") {|f| + @conf.keys.map {|cate_key| + cate_key.split('.', 2) + }.zip(@conf.values).group_by {|(section,key), val| + section + }.each {|section,cate_key_vals| + f.puts "[#{section}]" + cate_key_vals.each {|(section,key), val| + f.puts " #{key} = #{val}" + } + } + } + end + end end # class Config end # module TreasureData From ac3c8eaeaf5ea82aa5d2ec9c268bdbecb3ace4ef Mon Sep 17 00:00:00 2001 From: TAGOMORI Satoshi Date: Wed, 25 Dec 2019 17:18:43 +0900 Subject: [PATCH 3/5] add an internal class to manage overridden parameters, default loaders, etc --- lib/td/config.rb | 134 +++++++++++++++++++++++++++++++---------------- 1 file changed, 88 insertions(+), 46 deletions(-) diff --git a/lib/td/config.rb b/lib/td/config.rb index 1fde7e7e..61cdaa59 100644 --- a/lib/td/config.rb +++ b/lib/td/config.rb @@ -11,20 +11,82 @@ class ConfigParseError < ConfigError class Config # class variables @@path = ENV['TREASURE_DATA_CONFIG_PATH'] || ENV['TD_CONFIG_PATH'] || File.join(ENV['HOME'], '.td', 'td.conf') + @@global_config_entries = nil def self.read(path=Config.path, create=false) ConfigFile.new(path) end - @@apikey = ENV['TREASURE_DATA_API_KEY'] || ENV['TD_API_KEY'] - @@apikey = nil if @@apikey == "" - @@cl_apikey = false # flag to indicate whether an apikey has been provided through the command-line - @@endpoint = ENV['TREASURE_DATA_API_SERVER'] || ENV['TD_API_SERVER'] - @@endpoint = nil if @@endpoint == "" - @@cl_endpoint = false # flag to indicate whether an endpoint has been provided through the command-line - @@import_endpoint = ENV['TREASURE_DATA_API_IMPORT_SERVER'] || ENV['TD_API_IMPORT_SERVER'] - @@import_endpoint = nil if @@endpoint == "" - @@cl_import_endpoint = false # flag to indicate whether an endpoint has been provided through the command-line option + def self.configfile_entries + @@global_config_entries ||= ConfigFile.new(Config.path) + end + + class OptionEntry + attr_reader :name + + def initialize(name, envvar_names = [], configfile_entry_name = nil, &default_getter) + @name = name + @value = nil + @envvar_names = envvar_names + @configfile_entry_name = configfile_entry_name + @default_getter = default_getter + @command_line_override = false + end + + def override? + @command_line_override + end + + def get + return @value if @value + @envvar_names.each do |envvar| + value = ENV[envvar] + return value if value && value != "" + end + if @configfile_entry_name && value = Config.configfile_entries[@configfile_entry_name] + return value + end + if @default_getter + return @default_getter.call + end + nil + end + + def set(value) + @value = value + @command_line_override = true + end + end + + API_ENDPOINT_PATTERN = /\Aapi(-(?:staging|development))?(-[a-z0-9]+)?\.(connect\.)?(eu01\.)?treasuredata\.(com|co\.jp)\z/io + + def self.endpoint_hostname(endpoint) + endpoint.sub(%r[https?://], '').sub(%r!\:[0-9]+\z!, '') + end + + def self.endpoint_variant(endpoint_type, api_endpoint) + if API_ENDPOINT_PATTERN =~ endpoint_hostname(api_endpoint) + case endpoint_type + when :import + "https://api#{$1}-import#{$2}.#{$3}#{$4}treasuredata.#{$5}" + when :workflow + "https://api#{$1}-workflow#{$2}.#{$3}#{$4}treasuredata.#{$5}" + else + raise ConfigError, "Invalid endpoint type '#{endpoint_type}'" + end + else + raise ConfigError, "#{endpoint_type.to_s.capitalize} is not supported for '#{self.endpoint}'" + end + end + + @@apikey = OptionEntry.new(:apikey, ['TREASURE_DATA_API_KEY', 'TD_API_KEY'], 'account.apikey') + @@endpoint = OptionEntry.new(:endpoint, ['TREASURE_DATA_API_SERVER', 'TD_API_SERVER'], 'account.endpoint') + @@import_endpoint = OptionEntry.new(:import_endpoint) do + endpoint_variant(:import, @@endpoint.get) + end + @@workflow_endpoint = OptionEntry.new(:workflow_endpoint) do + endpoint_variant(:workflow, @@endpoint.get) + end @@secure = true @@retry_post_requests = false @@ -34,9 +96,9 @@ def self.path def self.path=(path) @@path = path + @@global_config_entries = nil end - def self.secure @@secure end @@ -54,72 +116,52 @@ def self.retry_post_requests=(retry_post_requests) end def self.apikey - @@apikey || Config.read['account.apikey'] + @@apikey.get end def self.apikey=(apikey) - @@apikey = apikey + @@apikey.set(apikey) end def self.cl_apikey - @@cl_apikey - end - - def self.cl_apikey=(flag) - @@cl_apikey = flag + @@apikey.override? end def self.endpoint - @@endpoint || Config.read['account.endpoint'] + @@endpoint.get end def self.endpoint=(endpoint) - @@endpoint = endpoint - end - - def self.endpoint_domain - (self.endpoint || 'api.treasuredata.com').sub(%r[https?://], '') + @@endpoint.set(endpoint) end def self.cl_endpoint - @@cl_endpoint - end - - def self.cl_endpoint=(flag) - @@cl_endpoint = flag + @@endpoint.override? end def self.import_endpoint - @@import_endpoint || Config.read['account.import_endpoint'] + @@import_endpoint.get end def self.import_endpoint=(endpoint) - @@import_endpoint = endpoint - end - - def self.cl_import_endpoint - @@cl_import_endpoint + @@import_endpoint.set(endpoint) end - def self.cl_import_endpoint=(flag) - @@cl_import_endpoint = flag + def self.workflow_endpoint + @@workflow_endpoint.get end - def self.workflow_endpoint - case self.endpoint_domain - when /\Aapi(-(?:staging|development))?(-[a-z0-9]+)?\.(connect\.)?(eu01\.)?treasuredata\.(com|co\.jp)\z/i - "https://api#{$1}-workflow#{$2}.#{$3}#{$4}treasuredata.#{$5}" - else - raise ConfigError, "Workflow is not supported for '#{self.endpoint}'" - end + def self.workflow_endpoint=(endpoint) + @@workflow_endpoint.set(endpoint) end # renders the apikey and endpoint options as a string for the helper commands def self.cl_options_string string = "" - string += "-k #{@@apikey} " if @@cl_apikey - string += "-e #{@@endpoint} " if @@cl_endpoint - string += "--import-endpoint #{@@import_endpoint} " if @@cl_import_endpoint + string += "-k #{@@apikey.get} " if @@apikey.override? + string += "-e #{@@endpoint.get} " if @@endpoint.override? + string += "--import-endpoint #{@@import_endpoint.get} " if @@import_endpoint.override? + string += "--workflow-endpoint #{@@workflow_endpoint.get}" if @@workflow_endpoint.override? string end From 693835d25387da84b02ea447778774afc69114ae Mon Sep 17 00:00:00 2001 From: TAGOMORI Satoshi Date: Wed, 25 Dec 2019 17:19:14 +0900 Subject: [PATCH 4/5] update code to use newer internal class/methods --- lib/td/command/account.rb | 2 +- lib/td/command/apikey.rb | 2 +- lib/td/command/runner.rb | 20 ++++++++++++++------ lib/td/command/server.rb | 2 +- lib/td/command/workflow.rb | 2 +- spec/td/command/workflow_spec.rb | 19 +++++++++++-------- spec/td/config_spec.rb | 22 ++++++++++++++++++++-- 7 files changed, 49 insertions(+), 20 deletions(-) diff --git a/lib/td/command/account.rb b/lib/td/command/account.rb index 6a352124..9fdfcf2f 100644 --- a/lib/td/command/account.rb +++ b/lib/td/command/account.rb @@ -93,7 +93,7 @@ def account(op) $stdout.puts "Authenticated successfully." - conf ||= Config.new + conf ||= Config::ConfigFile.new conf["account.user"] = user_name conf["account.apikey"] = client.apikey conf['account.endpoint'] = endpoint if endpoint diff --git a/lib/td/command/apikey.rb b/lib/td/command/apikey.rb index 3993d14f..c5967e1f 100644 --- a/lib/td/command/apikey.rb +++ b/lib/td/command/apikey.rb @@ -50,7 +50,7 @@ def apikey_set(op) end end - conf ||= Config.new + conf ||= Config::ConfigFile.new conf.delete("account.user") conf["account.apikey"] = apikey conf.save diff --git a/lib/td/command/runner.rb b/lib/td/command/runner.rb index dc0e078c..32a19de4 100644 --- a/lib/td/command/runner.rb +++ b/lib/td/command/runner.rb @@ -7,11 +7,12 @@ def initialize @apikey = nil @endpoint = nil @import_endpoint = nil + @workflow_endpoint = nil @prog_name = nil @insecure = false end - attr_accessor :apikey, :endpoint, :import_endpoint, :config_path, :prog_name, :insecure + attr_accessor :apikey, :endpoint, :import_endpoint, :workflow_endpoint, :config_path, :prog_name, :insecure def run(argv=ARGV) require 'td/version' @@ -74,7 +75,8 @@ def run(argv=ARGV) config_path = @config_path apikey = @apikey endpoint = @endpoint - import_endpoint = @import_endpoint || @endpoint + import_endpoint = @import_endpoint + workflow_endpoint = @workflow_endpoint insecure = nil $verbose = false #$debug = false @@ -96,12 +98,18 @@ def run(argv=ARGV) endpoint = e } - op.on('--import-endpoint API_IMPORT_SERVER', "specify the URL for API Import server to use (default: https://api-import.treasuredata.com).") { |e| + op.on('--import-endpoint IMPORT_SERVER', "specify the URL for Import server to use (default: https://api-import.treasuredata.com).") { |e| require 'td/command/common' Command.validate_api_endpoint(e) import_endpoint = e } + op.on('--workflow-endpoint WORKFLOW_SERVER', "specify the URL for Workflow server to use (default: https://api-workflow.treasuredata.com).") { |e| + require 'td/command/common' + Command.validate_api_endpoint(e) + workflow_endpoint = e + } + op.on('--insecure', "Insecure access: disable SSL (enabled by default)") {|b| insecure = true } @@ -142,15 +150,15 @@ def run(argv=ARGV) end if apikey Config.apikey = apikey - Config.cl_apikey = true end if endpoint Config.endpoint = endpoint - Config.cl_endpoint = true end if import_endpoint Config.import_endpoint = import_endpoint - Config.cl_import_endpoint = true + end + if workflow_endpoint + Config.workflow_endpoint = workflow_endpoint end if insecure Config.secure = false diff --git a/lib/td/command/server.rb b/lib/td/command/server.rb index dacba03d..d82b8044 100644 --- a/lib/td/command/server.rb +++ b/lib/td/command/server.rb @@ -26,7 +26,7 @@ def server_endpoint(op) begin conf = Config.read rescue ConfigError - conf = Config.new + conf = Config::ConfigFile.new end conf["account.endpoint"] = endpoint conf.save diff --git a/lib/td/command/workflow.rb b/lib/td/command/workflow.rb index a665aa5d..45382cf4 100644 --- a/lib/td/command/workflow.rb +++ b/lib/td/command/workflow.rb @@ -42,7 +42,7 @@ def workflow(op, capture_output=false, check_prereqs=true) "secrets.td.apikey = #{apikey}" ].join($/) + $/) cmd << '-Dio.digdag.standards.td.secrets.enabled=false' - cmd << "-Dconfig.td.default_endpoint=#{Config.endpoint_domain}" + cmd << "-Dconfig.td.default_endpoint=#{Config.endpoint_hostname(Config.endpoint)}" if workflow_endpoint.match(/\.connect\./i) cmd << '-Dclient.http.disable_direct_download=true' end diff --git a/spec/td/command/workflow_spec.rb b/spec/td/command/workflow_spec.rb index e91b57e1..3cf5ee37 100644 --- a/spec/td/command/workflow_spec.rb +++ b/spec/td/command/workflow_spec.rb @@ -329,7 +329,7 @@ def with_env(name, var) describe 'verify argument with mock' do let(:command) { Class.new { include TreasureData::Command }.new } let(:empty_option) { List::CommandParser.new("workflow", [], [], nil, [], true) } - let(:td_config){ TreasureData::Config.new } + let(:td_config){ TreasureData::Config::ConfigFile.new } let(:workflow_config){ Hash.new } let(:digdag_env){ Hash.new } let(:config_path){ nil } @@ -343,11 +343,9 @@ def with_env(name, var) end if apikey TreasureData::Config.apikey = apikey - TreasureData::Config.cl_apikey = true end if endpoint TreasureData::Config.endpoint = endpoint - TreasureData::Config.cl_endpoint = true end command_args.clear @@ -388,11 +386,16 @@ def with_env(name, var) let (:endpoint){ 'https://api.treasuredata.com' } it 'uses td.conf' do apikey = '1/deadbeaf' - TreasureData::Config.apikey = apikey # emulate to load from config file - op = List::CommandParser.new("workflow", [], [], nil, ['version'], true) - command.workflow(op, false, false) - expect(workflow_config).to eq({}) - expect(digdag_env['TREASURE_DATA_WORKFLOW_ENDPOINT']).to be_nil + begin + ENV['TD_API_KEY'] = apikey + # TreasureData::Config.apikey = apikey # emulate to load from config file + op = List::CommandParser.new("workflow", [], [], nil, ['version'], true) + command.workflow(op, false, false) + expect(workflow_config).to eq({}) + expect(digdag_env['TREASURE_DATA_WORKFLOW_ENDPOINT']).to be_nil + ensure + ENV.delete('TD_API_KEY') + end end end context 'endpoint: https://api.treasuredata.co.jp' do diff --git a/spec/td/config_spec.rb b/spec/td/config_spec.rb index 7452646f..5ad05b42 100644 --- a/spec/td/config_spec.rb +++ b/spec/td/config_spec.rb @@ -3,6 +3,25 @@ require 'td/config' describe TreasureData::Config do + context 'import_endpoint' do + before { TreasureData::Config.endpoint = api_endpoint } + subject { TreasureData::Config.import_endpoint } + context 'api.treasuredata.com' do + context 'works without schema' do + let(:api_endpoint){ 'api.treasuredata.com' } + it { is_expected.to eq 'https://api-import.treasuredata.com' } + end + context 'works with port number' do + let(:api_endpoint){ 'api.treasuredata.com:443' } + it { is_expected.to eq 'https://api-import.treasuredata.com' } + end + context 'works with https schema' do + let(:api_endpoint){ 'https://api.treasuredata.com' } + it { is_expected.to eq 'https://api-import.treasuredata.com' } + end + end + end + context 'workflow_endpoint' do before { TreasureData::Config.endpoint = api_endpoint } subject { TreasureData::Config.workflow_endpoint } @@ -85,8 +104,7 @@ f.close - config = TreasureData::Config.new - config.read(f.path) + config = TreasureData::Config.read(f.path) expect(config["section1.key"]).to eq "val" expect(config["section1.foo"]).to eq "bar" From bc6c7197a5fc7a1f104ccee574858fd11daa7711 Mon Sep 17 00:00:00 2001 From: TAGOMORI Satoshi Date: Mon, 6 Jan 2020 14:36:42 +0900 Subject: [PATCH 5/5] assume to use site:aws in default --- lib/td/config.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/td/config.rb b/lib/td/config.rb index 61cdaa59..befbe056 100644 --- a/lib/td/config.rb +++ b/lib/td/config.rb @@ -61,7 +61,7 @@ def set(value) API_ENDPOINT_PATTERN = /\Aapi(-(?:staging|development))?(-[a-z0-9]+)?\.(connect\.)?(eu01\.)?treasuredata\.(com|co\.jp)\z/io def self.endpoint_hostname(endpoint) - endpoint.sub(%r[https?://], '').sub(%r!\:[0-9]+\z!, '') + (endpoint || 'https://api.treasuredata.com').sub(%r[https?://], '').sub(%r!\:[0-9]+\z!, '') end def self.endpoint_variant(endpoint_type, api_endpoint)