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

Multi table import #118

Merged
merged 5 commits into from
Jun 16, 2020
Merged

Multi table import #118

merged 5 commits into from
Jun 16, 2020

Conversation

craigds
Copy link
Member

@craigds craigds commented Jun 15, 2020

This changes the init and import commands so they can import multiple tables at once.

Further, it changes the usage so that:

  • there's an --all-tables option
  • tables are now positional arguments for the import command both commands
  • any table can be renamed during import using source_table:dest_table
  • the existing optional (poorly named) DIRECTORY argument to import has been removed. It renamed tables, so is obsoleted by the above.
  • tables cannot be specified in the init --import command - it always imports all tables.
  • the existing optional DIRECTORY argument to the init has been moved to a --path PATH option. This was required to make table names unambiguous in that command. The command raises a helpful error if you attempt to init with a table name without passing --import, because that's now clearly a mis-usage.

todo:

tests:

  • add a test for multiple table import

Fixes #92 .

craigds added 2 commits June 15, 2020 19:15
This optional positional argument prevents us from using positional
arguments for other things in `init`.

Specifically, we want to specify (multiple) table names to import,
as positional arguments.
@craigds craigds requested review from rcoup and olsen232 June 15, 2020 07:45
@craigds craigds force-pushed the multi-table-import branch from 3b5c171 to 0c26930 Compare June 15, 2020 07:47
@craigds craigds force-pushed the multi-table-import branch from 0c26930 to 815db3a Compare June 15, 2020 21:29
Remove the ability to specify tables in `init` command. If you need it,
you can `init` and then `import` as a separate command.
@craigds craigds force-pushed the multi-table-import branch from 365c66c to 0213548 Compare June 16, 2020 00:59
@craigds
Copy link
Member Author

craigds commented Jun 16, 2020

In the latest commit, I changed tack after some offline feedback:

remove just the option to specify tables from sno init --import. ie, you can import in your init command, but you can only import a whole database and you can't rename tables. if you need more fine-grained control you have to use sno init and sno import seperately.

@rcoup agreed:

Agree that any advanced behaviours should use init + import separately.

The latest commit implements that behaviour - it's not possible to specify tables for the sno init --import command, and thus it was okay to reinstate the DIRECTORY argument for sno init as it no longer conflicted with positional table arguments.

sno/init.py Outdated Show resolved Hide resolved
sno/structure.py Outdated Show resolved Hide resolved
sno/structure.py Outdated Show resolved Hide resolved
@craigds craigds merged commit 1cd527d into master Jun 16, 2020
@craigds craigds deleted the multi-table-import branch June 16, 2020 08:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

import: Import multiple tables at once
2 participants