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

[CT-3177] [CLI] Raise deprecation warning for legacy MultiOption syntax #8758

Open
3 tasks done
jtcohen6 opened this issue Oct 3, 2023 · 0 comments
Open
3 tasks done
Labels
cli Impact: CLI tech_debt Behind-the-scenes changes, with little direct impact on end-user functionality

Comments

@jtcohen6
Copy link
Contributor

jtcohen6 commented Oct 3, 2023

Is this your first time submitting a feature request?

  • I have read the expectations for open source contributors
  • I have searched the existing issues, and I could not find an existing issue for this feature
  • I am requesting a straightforward extension of existing dbt functionality, rather than a Big Idea better suited to a discussion

Describe the feature

Thanks to #8598, dbt-core's "multi-option" parameters support three ways of passing in arguments:

$ dbt --option arg1 arg2
$ dbt --option "arg1 arg2"
$ dbt --option arg1 --option arg2

The first is legacy behavior, and not standard across most CLIs. We should officially deprecate this behavior, and raise a warning encouraging the user to opt for (2) or (3) instead. Probably (2) is the simplest & quickest way to migrate from (1).


This seems to be a potential spot:

                    if not done:
                        value_list.append(state.rargs.pop(0))
                        print("Legacy multi-option support is deprecated. Use quotes instead.")
$ dbt --quiet list --select one two
Legacy multi-option support is deprecated.
$ dbt --quiet list --select "one two"
$ dbt --quiet list --select one --select two

Technical trickiness:

  • This is almost certainly being resolved before we have a logger / event manager
  • If we want to provide the most helpful warning message possible, we should wait until we've collected all arguments, and then reconstruct the same thing with quotes

Describe alternatives you've considered

Not adding this deprecation warnings, allowing users to stub their toes when they encounter inconsistencies across CLIs / OSes

Who will this benefit?

Users of multiple dbt CLIs

Are you interested in contributing this feature?

No response

Anything else?

@jtcohen6 jtcohen6 added cli tech_debt Behind-the-scenes changes, with little direct impact on end-user functionality Impact: CLI labels Oct 3, 2023
@github-actions github-actions bot changed the title [CLI] Raise deprecation warning for legacy MultiOption syntax [CT-3177] [CLI] Raise deprecation warning for legacy MultiOption syntax Oct 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli Impact: CLI tech_debt Behind-the-scenes changes, with little direct impact on end-user functionality
Projects
None yet
Development

No branches or pull requests

1 participant