Use explicitly passed in arguments instead of args object in builder creation codepaths #22379
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 Jun 8, 2024
schrockn
force-pushed
the
refactor-builder-creation
branch
from
2819f71 to
51b04db
Compare
schrockn
changed the base branch from
multi-asset-check-compute_required-resources-func
to
rename-build-asset-outs
This was referenced Jun 8, 2024
schrockn
force-pushed
the
rename-build-asset-outs
branch
from
abb0f39 to
8c0018e
Compare
schrockn
force-pushed
the
refactor-builder-creation
branch
from
51b04db to
e543a9e
Compare
schrockn
force-pushed
the
rename-build-asset-outs
branch
from
8c0018e to
f2a3052
Compare
schrockn
force-pushed
the
refactor-builder-creation
branch
2 times, most recently
from
720a0cc to
1cf843d
Compare
schrockn
force-pushed
the
rename-build-asset-outs
branch
from
f2a3052 to
49b5c3d
Compare
schrockn
force-pushed
the
refactor-builder-creation
branch
from
1cf843d to
e12a356
Compare
schrockn
changed the title
schrockn
force-pushed
the
refactor-builder-creation
branch
from
2cfbe50 to
13657b8
Compare
This was referenced Jun 8, 2024
Closed
Merged
schrockn
force-pushed
the
rename-build-asset-outs
branch
2 times, most recently
from
8296360 to
d3b91f4
Compare
schrockn
force-pushed
the
refactor-builder-creation
branch
from
13657b8 to
3ebedaa
Compare
This was referenced Jun 9, 2024
This was referenced Jun 9, 2024
schrockn
force-pushed
the
refactor-builder-creation
branch
from
f9c1ce2 to
2362771
Compare
This was referenced Jun 10, 2024
sryza
approved these changes
Jun 12, 2024
| asset_out_map: Mapping[str, AssetOut], | ||
| asset_deps: Mapping[str, Set[AssetKey]], | ||
| upstream_asset_deps: Optional[Iterable[AssetDep]], | ||
| passed_args: DecoratorAssetsDefinitionBuilderArgs, |
There was a problem hiding this comment.
Passing this through along with the other stuff feels a little weird. Maybe the check should just happen outside it?
There was a problem hiding this comment.
I agree that this is weird and lame. A bunch of stuff happening in this code path and workstream and this will go away anyways, so I'm not going to resolve it in this PR.
schrockn
force-pushed
the
refactor-builder-creation
branch
from
2362771 to
2df8109
Compare
This was referenced Jun 12, 2024
cmpadden
pushed a commit
that referenced
this pull request
Jun 13, 2024
…creation codepaths (#22379) ## Summary & Motivation I am moving toward using DecoratorAssetsDefinitionBuilder in more codepaths (asset checks, graph assets) and things are going to get a little gnarly. In this case I want to make it explicit which variables are used in these factory methods rather than relying on the grabbag "args" object. ## How I Tested These Changes BK
danielgafni
pushed a commit
to danielgafni/dagster
that referenced
this pull request
Jun 18, 2024
…creation codepaths (dagster-io#22379) ## Summary & Motivation I am moving toward using DecoratorAssetsDefinitionBuilder in more codepaths (asset checks, graph assets) and things are going to get a little gnarly. In this case I want to make it explicit which variables are used in these factory methods rather than relying on the grabbag "args" object. ## How I Tested These Changes BK
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
I am moving toward using DecoratorAssetsDefinitionBuilder in more codepaths (asset checks, graph assets) and things are going to get a little gnarly.
In this case I want to make it explicit which variables are used in these factory methods rather than relying on the grabbag "args" object.
How I Tested These Changes
BK