-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
[amp-refactor][?/n] Add asset_condition argument to AssetsDefinition and friends #18802
Closed
OwenKephart
wants to merge
1
commit into
master
from
12-18-Add_asset_condition_argument_to_AssetsDefinition_etal
Closed
[amp-refactor][?/n] Add asset_condition argument to AssetsDefinition and friends #18802
OwenKephart
wants to merge
1
commit into
master
from
12-18-Add_asset_condition_argument_to_AssetsDefinition_etal
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 Dec 18, 2023
OwenKephart
force-pushed
the
12-18-Add_asset_condition_argument_to_AssetsDefinition_etal
branch
from
December 18, 2023 22:59
1f38134
to
8ca46fd
Compare
OwenKephart
force-pushed
the
12-15-Better_cursor_handling
branch
from
December 19, 2023 23:19
0dd3369
to
cf30630
Compare
OwenKephart
force-pushed
the
12-18-Add_asset_condition_argument_to_AssetsDefinition_etal
branch
from
December 19, 2023 23:19
8ca46fd
to
0d547d4
Compare
OwenKephart
force-pushed
the
12-15-Better_cursor_handling
branch
from
December 21, 2023 20:49
cf30630
to
7f8a5f4
Compare
OwenKephart
force-pushed
the
12-15-Better_cursor_handling
branch
from
December 29, 2023 15:30
7f8a5f4
to
a8629aa
Compare
OwenKephart
force-pushed
the
12-18-Add_asset_condition_argument_to_AssetsDefinition_etal
branch
from
December 29, 2023 15:30
0d547d4
to
88f5a9a
Compare
OwenKephart
force-pushed
the
12-15-Better_cursor_handling
branch
from
December 29, 2023 18:52
a8629aa
to
21a936e
Compare
OwenKephart
force-pushed
the
12-18-Add_asset_condition_argument_to_AssetsDefinition_etal
branch
from
December 29, 2023 18:52
88f5a9a
to
a3e232e
Compare
OwenKephart
force-pushed
the
12-15-Better_cursor_handling
branch
from
December 29, 2023 20:32
21a936e
to
d602281
Compare
OwenKephart
force-pushed
the
12-18-Add_asset_condition_argument_to_AssetsDefinition_etal
branch
from
December 29, 2023 20:32
a3e232e
to
abc3927
Compare
This was referenced Dec 29, 2023
OwenKephart
force-pushed
the
12-15-Better_cursor_handling
branch
from
December 29, 2023 20:58
d602281
to
063c290
Compare
OwenKephart
force-pushed
the
12-18-Add_asset_condition_argument_to_AssetsDefinition_etal
branch
from
December 29, 2023 20:58
abc3927
to
7215ad2
Compare
OwenKephart
changed the title
[amp-refactor][10/n] Add asset_condition argument to AssetsDefinition and friends
[amp-refactor][?/n] Add asset_condition argument to AssetsDefinition and friends
Dec 29, 2023
OwenKephart
force-pushed
the
12-15-Better_cursor_handling
branch
4 times, most recently
from
January 4, 2024 13:46
5e0751f
to
7b1680f
Compare
OwenKephart
force-pushed
the
12-15-Better_cursor_handling
branch
from
January 4, 2024 15:59
7b1680f
to
8a51001
Compare
OwenKephart
force-pushed
the
12-15-Better_cursor_handling
branch
from
January 8, 2024 20:39
8a51001
to
55ffe29
Compare
OwenKephart
force-pushed
the
12-18-Add_asset_condition_argument_to_AssetsDefinition_etal
branch
from
January 8, 2024 20:40
7215ad2
to
d0ba4df
Compare
OwenKephart
force-pushed
the
12-15-Better_cursor_handling
branch
from
January 9, 2024 17:08
55ffe29
to
aee24f6
Compare
OwenKephart
force-pushed
the
12-18-Add_asset_condition_argument_to_AssetsDefinition_etal
branch
from
January 9, 2024 17:08
d0ba4df
to
a943b62
Compare
OwenKephart
force-pushed
the
12-15-Better_cursor_handling
branch
from
January 11, 2024 22:21
aee24f6
to
794ef38
Compare
OwenKephart
force-pushed
the
12-18-Add_asset_condition_argument_to_AssetsDefinition_etal
branch
from
January 11, 2024 22:21
a943b62
to
5d3635c
Compare
OwenKephart
force-pushed
the
12-15-Better_cursor_handling
branch
from
January 11, 2024 23:09
794ef38
to
a666900
Compare
OwenKephart
force-pushed
the
12-18-Add_asset_condition_argument_to_AssetsDefinition_etal
branch
from
January 11, 2024 23:09
5d3635c
to
e2eef99
Compare
OwenKephart
force-pushed
the
12-15-Better_cursor_handling
branch
2 times, most recently
from
January 12, 2024 13:58
7ba378c
to
ba6e90e
Compare
OwenKephart
force-pushed
the
12-18-Add_asset_condition_argument_to_AssetsDefinition_etal
branch
from
January 12, 2024 13:58
e2eef99
to
e5551f7
Compare
OwenKephart
force-pushed
the
12-15-Better_cursor_handling
branch
from
January 12, 2024 16:56
ba6e90e
to
3be95a8
Compare
OwenKephart
force-pushed
the
12-18-Add_asset_condition_argument_to_AssetsDefinition_etal
branch
from
January 12, 2024 16:56
e5551f7
to
25cabc4
Compare
OwenKephart
force-pushed
the
12-15-Better_cursor_handling
branch
from
January 12, 2024 18:01
3be95a8
to
808ad3c
Compare
OwenKephart
force-pushed
the
12-18-Add_asset_condition_argument_to_AssetsDefinition_etal
branch
from
January 12, 2024 18:02
25cabc4
to
8ea75c2
Compare
OwenKephart
force-pushed
the
12-15-Better_cursor_handling
branch
3 times, most recently
from
January 19, 2024 21:34
65b4079
to
e0f49ab
Compare
OwenKephart
force-pushed
the
12-18-Add_asset_condition_argument_to_AssetsDefinition_etal
branch
from
February 7, 2024 23:21
8ea75c2
to
41d3d8c
Compare
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
The plan is to allow users to supply AssetCondition objects instead of AutoMaterializePolicies. This PR makes that happen by turning the AutoMaterializePolicy class into a shell whose only purpose is to generate AssetConditions.
This meant that it had to be removed at the storage layer as well, as we want external asset nodes to be able to store regular AssetConditions. Luckily, it's fairly easy to ensure that AutoMaterializePolicies get deserialized as AssetConditions, as we already had some customized serialization going on in that part of the world.
How I Tested These Changes