[DA] Fix evaluation history UI #22859
Merged
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
alangenfeld
reviewed
Jul 8, 2024
|
@alangenfeld This intermediate state should only exist for a few releases (until we figure out a more complete redesign) and should only make an impact for people using the new APIs, so my feeling is that the risk level is low (+ it solves an immediate blocking issue) But @salazarm if you have thoughts to the contrary definitely let me know |
|
sounds reasonable to me if this is an intermediate state |
alangenfeld
approved these changes
Jul 8, 2024
| elif maybe_subset.is_partitioned: | ||
| return GrapheneAssetSubset(maybe_subset) | ||
|
|
||
| # TODO: Remove on redesign |
| @@ -187,7 +197,13 @@ def __init__(self, evaluation: AssetConditionEvaluation, partition_key: str): | |||
| self._evaluation = evaluation | |||
| self._partition_key = partition_key | |||
|
|
|||
| if partition_key in evaluation.true_subset.subset_value: | |||
| if not evaluation.true_subset.is_partitioned: | |||
| # TODO: Remove on redesign | |||
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
Resolves: #22787
The new Declarative Automation framework allows conditions which are evaluated against parents of a given asset. Those parents do not necessarily have the same partitions definition as the child.
The current UI was designed with the expectation that all sub-evaluations would reference the same PartitionsDefinition, and so results in errors when we (e.g.) try to construct a
GraphenePartitionedAssetConditionEvaluationNodefrom an evaluation that was actually executed against an unpartitioned asset.We are working on a larger redesign which should help remove this assumption from the core UI model, but for now we can prevent these errors by making it possible to construct a
...PartitionedEvaluationNodefrom an unpartitioned evaluation and vice-versa.The end result is a comprehensible UI without errors, although compromises needed to be made (e.g. an unpartitioned row in the partitioned UI ends up being represented as having a "partition key" of the string "None"). This feels like an acceptable state for the short term.
How I Tested These Changes