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-659] [Feature] Add a better name for flags.WHICH #5262

Closed
1 task done
SOVALINUX opened this issue May 17, 2022 · 3 comments
Closed
1 task done

[CT-659] [Feature] Add a better name for flags.WHICH #5262

SOVALINUX opened this issue May 17, 2022 · 3 comments
Labels
artifacts enhancement New feature or request logging stale Issues that have gone stale tech_debt Behind-the-scenes changes, with little direct impact on end-user functionality

Comments

@SOVALINUX
Copy link

SOVALINUX commented May 17, 2022

Is this your first time opening an issue?

Describe the Feature

We have different dbt commands (run, test, compile, docs, etc.) and refer them as "commands"
However the variable in the context storing this information called "flags.WHICH", what is clear for developer community, but might be confusing for users
Suggestion is to have another flag: flags.EXECUTED_COMMAND or flags.COMMAND to store the same as flags.WHICH
Deprecate flags.WHICH and potentially remove it in dbt 2.0

Describe alternatives you've considered

I even made a PR thinking that flags.WHICH doesn't exist :)

Who will this benefit?

everyone who're doing hooks and unit testing

Are you interested in contributing this feature?

I think I can

Anything else?

This feature is a spin-off from #5251

@SOVALINUX SOVALINUX added enhancement New feature or request triage labels May 17, 2022
@github-actions github-actions bot changed the title [Feature] Add a better name for flags.WHICH [CT-659] [Feature] Add a better name for flags.WHICH May 17, 2022
@jtcohen6 jtcohen6 added artifacts tech_debt Behind-the-scenes changes, with little direct impact on end-user functionality Team:Execution logging and removed triage labels May 20, 2022
@jtcohen6
Copy link
Contributor

jtcohen6 commented May 20, 2022

@SOVALINUX Thanks for opening! Let's definitely pick a new name for this, one that makes more sense to end users and metadata consumers :)

Tagging this as both Language + Execution because:

  • it relates to the tightly coupled config at dbt initiatlization that we've been discussing
  • it has implications for dbt project code and metadata interfaces
  • I think the choice of the word "which" isn't actually ours — it's argparse's built-in word for identifying which subparser (=subcommand) is being used (reading here) — does that sound right?

If the last bullet is indeed correct, it also makes me think:

  • this is something that would (or should) come up naturally if/when we take up work that moves us away from argparse, toward a better CLI interface
  • this is work we should take on ourselves, as part of that effort

Backwards compatibility considerations

  1. flags module, which is exposed to the Jinja context for end users writing project code. This should be relatively easy: flags.WHICH could just redirect to the new name.
  2. Metadata consumers: structured logging events that include args, i.e. MainReportArgs, and artifacts that include args, i.e. run_results.json (as documented). I think this would be trickier, without lots of duplication.

@github-actions
Copy link
Contributor

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 remove the stale label or comment on the issue, or it will be closed in 7 days.

@github-actions github-actions bot added the stale Issues that have gone stale label Nov 17, 2022
@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; add a comment to notify the maintainers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
artifacts enhancement New feature or request logging stale Issues that have gone stale tech_debt Behind-the-scenes changes, with little direct impact on end-user functionality
Projects
None yet
Development

No branches or pull requests

2 participants