Skip to content

Conversation

@takkanm
Copy link
Contributor

@takkanm takkanm commented Aug 6, 2015

now --database and --table is obsolete

ref #67

now `--database` and `--table` is obsolete
@takkanm takkanm self-assigned this Aug 11, 2015
@takkanm
Copy link
Contributor Author

takkanm commented Aug 19, 2015

@nahi @muga please, review it.

@muga muga changed the title connector:issue recommend use arugument connector:issue deprecates --database, --table and --time-column options Aug 19, 2015
@frsyuki
Copy link
Member

frsyuki commented Aug 19, 2015

Discussion at #67 includes an idea to include database and table names in out: section of config file. And I think that is better. If we take that way, adding this code will create another obsoleted code (which can't be removed in fact) in the future.

@muga Do you think it's better to set db and table names at cmdline argument?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is not backward compatible. This makes existent automated shell scripts failed which are using --database and --table options.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry. I'll fix it.

@takkanm
Copy link
Contributor Author

takkanm commented Aug 20, 2015

I fixed following items.

  • allow no argument for database, table, time-column, if config.yml include these option.
  • include database, table, and time-column option in config['out'] for td-api.
  • some refactoring.

@muga
Copy link
Member

muga commented Aug 22, 2015

Thank you for your working:-) The fix requires platform side change. Please talk about it later.

@takkanm
Copy link
Contributor Author

takkanm commented Aug 25, 2015

I removed argument option(database, table) . Because, these options should write to configuration file.
d2e3ad4

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants