feat: allow passing config for dagster asset materialize cli #26069
Conversation
jimmyshah
changed the title
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| with instance_for_test() as instance: | ||
| runner = CliRunner() | ||
|
|
||
| result = runner.invoke(asset_materialize_command, ["--select", "asset_with_config", "--cli-config-json", "{\"some_prop\": \"foo\"}"]) |
There was a problem hiding this comment.
The command line argument in the test should be --config-json to match the option defined in asset.py. The test is currently using --cli-config-json, which will cause the test to fail.
Spotted by Graphite Reviewer
Is this helpful? React 👍 or 👎 to let us know.
There was a problem hiding this comment.
Hey @jimmyshah !! Overall looks good, two small comments, otherwise this is basically good to go
| @@ -3,9 +3,11 @@ | |||
| import click | |||
|
|
|||
| import dagster._check as check | |||
| from dagster._cli.job import get_config_from_args | |||
There was a problem hiding this comment.
this one should be moved into dagster._cli.workspace.cli_target, I believe
| #assert "some_prop:foo" in result.output | ||
| #assert instance.get_latest_materialization_event(AssetKey("asset_with_config")) is not None |
There was a problem hiding this comment.
the thing that's important to test here is that the config actually made its way into the asset (this can be accomplished by asserting the config value within the body of asset_with_config:
@asset
def asset_with_config(context: AssetExecutionContext, config: MyConfig):
assert config.some_prop == "foo"
context.log.info(f"some_prop:{config.some_prop}"
Summary & Motivation
I was browsing the repo for good first issues as a new contributor, and found #16802 interesting. It looks like @sam-goodwin's PR #20979 is nearly complete, except for a change needed for one of the imports. I used that PR's solution, but also resolved the import issue.
How I Tested These Changes
I am currently running
python -m pytest python_modules/dagster/dagster_teststo run the tests locally, though it is taking some time to complete. The tests appear to be passing so far!