[cli-refactor] Type and validate python_pointer_opts at entry point #27501
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Feb 3, 2025
smackesey
commented
Feb 3, 2025
| container_context: Optional[str] = None, | ||
| location_name: Optional[str] = None, | ||
| instance_ref=None, | ||
| inject_env_vars_from_instance: bool = False, |
There was a problem hiding this comment.
These defaults are unnecessary and misleading-- click will always call the function with values for all args, so no default is ever used.
smackesey
changed the base branch from
sean/workspace/consolidate-job-target
to
graphite-base/27501
smackesey
force-pushed
the
graphite-base/27501
branch
from
5d7ae2e to
5ab09e4
Compare
smackesey
force-pushed
the
sean/workspace/python-pointer-opts
branch
from
a82982f to
2e6cb01
Compare
smackesey
force-pushed
the
graphite-base/27501
branch
from
5ab09e4 to
a58be30
Compare
smackesey
force-pushed
the
sean/workspace/python-pointer-opts
branch
from
2e6cb01 to
ed80169
Compare
smackesey
commented
Feb 3, 2025
| ) -> None: | ||
| python_pointer_opts = PythonPointerOpts.extract_from_cli_options(other_opts) | ||
| assert_no_remaining_opts(other_opts) |
There was a problem hiding this comment.
assert_no_remaining_opts ensures we didn't forget to list any other options in the signature-- all options are accounted for by either being explicitly listed, or extracted from the other_opts dict into a typed object.
smackesey
force-pushed
the
sean/workspace/python-pointer-opts
branch
3 times, most recently
from
7563e7b to
83a1ae9
Compare
smackesey
force-pushed
the
graphite-base/27501
branch
from
a58be30 to
9c0b023
Compare
smackesey
force-pushed
the
graphite-base/27501
branch
from
9c0b023 to
0b89790
Compare
smackesey
force-pushed
the
sean/workspace/python-pointer-opts
branch
2 times, most recently
from
0ae237a to
cfd3e5d
Compare
smackesey
force-pushed
the
graphite-base/27501
branch
from
0b89790 to
97522d5
Compare
schrockn
approved these changes
Feb 3, 2025
smackesey
force-pushed
the
sean/workspace/python-pointer-opts
branch
from
cfd3e5d to
6658a41
Compare
benpankow
added a commit
that referenced
this pull request
Feb 4, 2025
## Summary #27501 introduced a change where we must pass the working directory to `dagster assets materialize` - adds that to our dg docs tests
smackesey
added a commit
that referenced
this pull request
Feb 10, 2025
## Summary & Motivation #27501 added typing and validation of args for all CLI entry points that accept a "python pointer" (i.e. pointer to a single code location). This PR continues that theme for all commands that accept a workspace specification. All commands that operate on a workspace now immediately parse passed workspace-specifyng options into the new typed `WorkspaceOpts` object. All these commands have also had their signatures cleaned up. Many of the helper methods used within these commands (which previously operated on untyped bags of "kwargs") have also been refactored. ## How I Tested These Changes Existing test suite (with some refactors to handle new `WorkspaceOpts`).
LoHertel
pushed a commit
to LoHertel/dagster
that referenced
this pull request
Feb 11, 2025
…agster-io#27501) ## Summary & Motivation First PR in the stack that replaces passing around "kwargs" with entry point validation and typing. - Add a `PythonPointerOpts` value object corresponding to the `@python_pointer_opts` decorator. - Wherever `@python_pointer_opts` is used: - Immediately parse the untyped bag of cli_opts into a `PythonPointerOpts` object. - Generally clean up the signatures of the targeted click commands, ensuring all other opts are passed as arguments instead of inside the generic bag of CLI opts. ## How I Tested These Changes Existing test suite.
LoHertel
pushed a commit
to LoHertel/dagster
that referenced
this pull request
Feb 11, 2025
## Summary dagster-io#27501 introduced a change where we must pass the working directory to `dagster assets materialize` - adds that to our dg docs tests
LoHertel
pushed a commit
to LoHertel/dagster
that referenced
this pull request
Feb 11, 2025
dagster-io#27559) ## Summary & Motivation dagster-io#27501 introduced a regression in setting the working directory of code pointers correctly for certain entry points-- this fixes it. ## How I Tested These Changes Docs beta snippet that was failing now passes.
LoHertel
pushed a commit
to LoHertel/dagster
that referenced
this pull request
Feb 11, 2025
…er-io#27540) ## Summary & Motivation dagster-io#27501 added typing and validation of args for all CLI entry points that accept a "python pointer" (i.e. pointer to a single code location). This PR continues that theme for all commands that accept a workspace specification. All commands that operate on a workspace now immediately parse passed workspace-specifyng options into the new typed `WorkspaceOpts` object. All these commands have also had their signatures cleaned up. Many of the helper methods used within these commands (which previously operated on untyped bags of "kwargs") have also been refactored. ## How I Tested These Changes Existing test suite (with some refactors to handle new `WorkspaceOpts`).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary & Motivation
First PR in the stack that replaces passing around "kwargs" with entry point validation and typing.
PythonPointerOptsvalue object corresponding to the@python_pointer_optsdecorator.@python_pointer_optsis used:PythonPointerOptsobject.How I Tested These Changes
Existing test suite.