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

fix: Remove JSON arguments in CLI #247

Merged
merged 11 commits into from
May 27, 2021
Merged

fix: Remove JSON arguments in CLI #247

merged 11 commits into from
May 27, 2021

Conversation

nehanene15
Copy link
Collaborator

Removes JSON arguments in CLI. This fix is backwards compatible for JSON inputs if needed.
Includes README and docs/examples updates.

  • Updated --tbls flag to accept tables in the form schema:table:target_table
  • Updated aggregation flags to accept comma-separated lists
  • Created --bq-result-handler which accepts project_id.dataset.table for result output
    • The original --rc flag is still supported. I created a separate flag for BigQuery so we can add new result handlers in the future with their own configuration parameters (in case they require more inputs than project_id and table_id).
  • Updated --filters flag to accept string values

Closes #241

@dhercher
Copy link
Collaborator

dhercher commented May 6, 2021

I'm worried about this change, particularly to the tables. It removes the flexibility to select non-matching schemas on source and target. I personally use that feature quite often.

I'm worried about this being a breaking change, perhaps it doesn't need to be?

@nehanene15
Copy link
Collaborator Author

I'm worried about this change, particularly to the tables. It removes the flexibility to select non-matching schemas on source and target. I personally use that feature quite often.

I'm worried about this being a breaking change, perhaps it doesn't need to be?

Updated the PR to use the '=' operator mapping for tables. A table can be provided like so:
source_schema.source_table=target_schema.target_table

"--count",
'["tripduration","start_station_name"]',
"tripduration,start_station_name",
Copy link
Collaborator

Choose a reason for hiding this comment

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

lets leave this one as JSON formatting to test backwards compat?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done 👍

@nehanene15 nehanene15 merged commit 5a309f7 into develop May 27, 2021
@nehanene15 nehanene15 deleted the issue241-cli branch May 27, 2021 14:49
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.

Update JSON CLI inputs to be more readable
2 participants