Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

connector:issue requires long --database and --table arguments #67

Open
frsyuki opened this issue Apr 29, 2015 · 5 comments
Open

connector:issue requires long --database and --table arguments #67

frsyuki opened this issue Apr 29, 2015 · 5 comments

Comments

@frsyuki
Copy link
Member

frsyuki commented Apr 29, 2015

but other commands like bulk_import or import uses just <db> <table> arguments. Can't we use this way instead?

@mcaramello
Copy link
Contributor

I vote for that too. I wondered about the same yesterday.

@muga
Copy link
Member

muga commented Apr 30, 2015

Sounds good. As another suggestion, I think that it's better that users can declare configuration file for connector:issue.

in:
    type: s3
    parser:
        type: csv
        ...
out:
    database: mydb
    table: mytable
    time_column: updated_at

Key points:

  • Users can specify in: attribute. Current config has config: attribute.
  • Users can add out: attribute instead of database and table arguments.

@nahi
Copy link
Member

nahi commented May 14, 2015

@muga Thanks, in: and out: should be handled by #73

'td connector:issue config db table' is easy to implement like the following. But I'm unsure it's needed now because we can't remove --database and --table already.

% git diff lib
diff --git a/lib/td/command/connector.rb b/lib/td/command/connector.rb
index efbf03b..84653fe 100644
--- a/lib/td/command/connector.rb
+++ b/lib/td/command/connector.rb
@@ -109,7 +109,7 @@ module Command
     op.on('-w', '--wait', 'wait for finishing the job', TrueClass) { |b| wait = b }
     op.on('-x', '--exclude', 'do not automatically retrieve the job result', TrueClass) { |b| exclude = b }

-    config_file = op.cmd_parse
+    config_file, database, table = op.cmd_parse

     required('--database', database)
     required('--table', table)
diff --git a/lib/td/command/list.rb b/lib/td/command/list.rb
index 97e7850..f63864e 100644
--- a/lib/td/command/list.rb
+++ b/lib/td/command/list.rb
@@ -335,7 +335,7 @@ module List
   add_list 'connector:guess', %w[config?], 'Run guess to generate connector config file', ['connector:guess td-bulkload.yml', 'connector:guess --access-id s3accessId --access-secret s3AccessKey --source https://s3.amazonaws.com/bucketname/path/prefix --database connector_database --table connector_table']
   add_list 'connector:preview', %w[config], 'Show preview of connector execution', ['connector:preview td-bulkload.yml']

-  add_list 'connector:issue', %w[config], 'Run one time connector execution', ['connector:issue td-bulkload.yml']
+  add_list 'connector:issue', %w[config db? table?], 'Run one time connector execution', ['connector:issue td-bulkload.yml']

   add_list 'connector:list', %w[], 'Show list of connector sessions', ['connector:list']
   add_list 'connector:create', %w[name cron database table config], 'Create new connector session', ['connector:create connector1 "0 * * * *" connector_database connector_table td-bulkload.yml']

@frsyuki
Copy link
Member Author

frsyuki commented Aug 19, 2015

Using config file would be the best idea as @muga describes. Because config file can be consistent with embulk-output-td, which makes it easy to migrate between TD data connector and local embulk.

However, setting database and table names in out: section needs API changes. This issue will be blocked until it's implemented on service-side.

@muga
Copy link
Member

muga commented Aug 19, 2015

For example, current connector:issue command is executed like:

$ cat config.yml
in:
  type: s3
  bucket: my_bucket
  ...
out:
  mode: replace

$ td connector:issue config.yml --database my_db --table my_table --time-column datetime

The suggestion on this ticket is following:

$ cat config.yml
in:
  type: s3
  bucket: my_bucket
  ...
out:
  database: my_db
  table: my_table
  time_column: datetime
  mode: replace

$ td connector:issue config.yml

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants