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

Refactor CLI to Command Pattern #279

Closed
ryanmcdowell opened this issue Jul 11, 2021 · 0 comments · Fixed by #303
Closed

Refactor CLI to Command Pattern #279

ryanmcdowell opened this issue Jul 11, 2021 · 0 comments · Fixed by #303
Assignees
Labels
priority: p1 High priority. Fix may be included in the next release. Release Next release candidate type: feature request 'Nice-to-have' improvement, new feature or different behavior or design.

Comments

@ryanmcdowell
Copy link
Collaborator

As the CLI has grown, there's many options that don't make sense to be specified together however they all exist for the same command (run). For example, --grouped-columns is an optional arg to the run command but it's only valid for a specific type of validation GroupedColumn. Same with --primary-keys and Row (#277).

To support future growth of the CLI and to simplify usage, I propose each validation becomes it's own top-level command. This would change the command-line from:

data-validation run [ARGS]

to

# Technically Column & Grouped Column is the same command with different opts 
# specified at runtime (e.g. the presence of a groupby key).
data-validation validate column [ARGS] 
data-validation validate schema [ARGS]
data-validation validate row [ARGS]

This change could be made backward compatible because the run command can continue to exist but now each command can be separated out into their own module with only the options relevant to that command present and inheriting from a validation command class (Command Design Pattern). This is similar to how we designed the gcloud SDK.

@freedomofnet freedomofnet added priority: p2 Medium priority. Fix may not be included in next release (e.g. minor documentation, cleanup) type: feature request 'Nice-to-have' improvement, new feature or different behavior or design. labels Jul 14, 2021
@nehanene15 nehanene15 self-assigned this Aug 11, 2021
@nehanene15 nehanene15 added priority: p1 High priority. Fix may be included in the next release. Release Next release candidate and removed priority: p2 Medium priority. Fix may not be included in next release (e.g. minor documentation, cleanup) labels Aug 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p1 High priority. Fix may be included in the next release. Release Next release candidate type: feature request 'Nice-to-have' improvement, new feature or different behavior or design.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants