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-1334] [Feature] Add entire dbt command under single to invocation_args_dict #6051

Closed
3 tasks done
jared-rimmer opened this issue Oct 12, 2022 · 5 comments · Fixed by #7939
Closed
3 tasks done
Labels
cli enhancement New feature or request

Comments

@jared-rimmer
Copy link
Contributor

jared-rimmer commented Oct 12, 2022

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

I previously implemented adding invocation args dict to the ProviderContext Class

This returns a JSON object that looks like the following:

{'write_json': True, 'use_colors': True, 'printer_width': 80, 'version_check': True, 'partial_parse': True, 'static_parser': True, 'profiles_dir': '/Users/jerco/.dbt', 'send_anonymous_usage_stats': False, 'event_buffer_size': 100000, 'quiet': False, 'no_print': False, 'parse_only': False, 'which': 'compile', 'rpc_method': 'compile', 'indirect_selection': 'eager', 'select' : ['+jercos_awesome_model'}

I would like to extend this to add a single key with the full dbt command as the value:

{'dbt_invocation_command': 'dbt run --select jercos_awesome_model'}

rather than having the flags passed to dbt available under different keys as they currently are:

{'select': ['+jercos_awesome_model'], 'exclude': ['jareds_terrible_model}

Describe alternatives you've considered

  • Creating a macro that joins all the elements back together again
  • Rewriting the implemention in dbt-core to do this joining when invocation_args_dict is called

I'm a little reluctant to do this joining back together as whenever a new flag is added / removed I think it would require a code update and release

Who will this benefit?

  • Users of the dbt_artifacts package
  • Anyone that wants to track the full invocation argument for whatever purposes they wish

Are you interested in contributing this feature?

Yes

Anything else?

No response

@jared-rimmer jared-rimmer added enhancement New feature or request triage labels Oct 12, 2022
@github-actions github-actions bot changed the title [Feature] Add entire dbt command under single to invocation_args_dict [CT-1334] [Feature] Add entire dbt command under single to invocation_args_dict Oct 12, 2022
@jtcohen6
Copy link
Contributor

@jared-rimmer Cool idea! I can see the benefit of: "Run exactly this CLI command, and you're guaranteed to be using exactly the same config as this invocation."

I could be mistaken, but I don't think we have an elegant way to convert a set of resolved flags/configs back into a set of CLI flags — e.g. to figure out that 'use_colors': False should mean --no-use-colors.

Good news: We are working on a new CLI, powered by click, with more programmatic definition of each flag/parameter. I see two options here:

  • Add a to_cli_params method of the Flags class, so that we can serialize (?) a Flags object as a CLI string
  • Allow the dbt-core CLI to accept a dictionary of flags (!), so that there's no need to perform the dict-to-string conversion: dbt --flag-dict {'write_json': True, ...}

@iknox-fa If you happen to have a moment, I'd be curious to hear your thoughts on which of those you prefer (or a third option I haven't thought of)

jercos_awesome_model

❤️

@jtcohen6 jtcohen6 added cli and removed triage labels Oct 30, 2022
@iknox-fa
Copy link
Contributor

iknox-fa commented Dec 4, 2022

I could be mistaken, but I don't think we have an elegant way to convert a set of resolved flags/configs back into a set of CLI flags — e.g. to figure out that 'use_colors': False should mean --no-use-colors.

You're correct, we don't have a way to do this and given the complexity of the various factors that might modify the flags, it's probably not a good idea to try to create a system to do so, but luckily we don't have to.

Either of your solutions are easy enough to implement (in fact, I don't see a reason not to do both).

  • If we want to track the actual string provided on the cli we should simply add it to flags from sys.argv[1:]. a single line change for the click side of things and probably not much more than that for the legacy flags module.
  • Adding a --flag-dict flag would also be fairly easy, although more complex than option 1. The main issue would be that the parameters provided would have to be injected into a click context which was then forwarded to the necessary subcommand.

@github-actions
Copy link
Contributor

github-actions bot commented Jun 3, 2023

This issue has been marked as Stale because it has been open for 180 days with no activity. If you would like the issue to remain open, please comment on the issue or else it will be closed in 7 days.

@github-actions github-actions bot added the stale Issues that have gone stale label Jun 3, 2023
@github-actions
Copy link
Contributor

Although we are closing this issue as stale, it's not gone forever. Issues can be reopened if there is renewed community interest. Just add a comment to notify the maintainers.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jun 11, 2023
@NiallRees
Copy link
Contributor

NiallRees commented Jun 23, 2023

Mind reopening this one @jtcohen6/@iknox-fa? Got a PR here: #7939

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants