Skip to content
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

Automation UI not working with new AutomationConditions #22787

Closed
ion-elgreco opened this issue Jul 1, 2024 · 6 comments · Fixed by #22859
Closed

Automation UI not working with new AutomationConditions #22787

ion-elgreco opened this issue Jul 1, 2024 · 6 comments · Fixed by #22859
Labels
area: auto-materialize Related to Auto Materialization type: bug Something isn't working

Comments

@ion-elgreco
Copy link
Contributor

Dagster version

1.7.12

What's the issue?

I created a new autoMaterializePolicy using the new AutomationConditions, since I needed some more flexibility of the rules.

However the UI doesn't work anymore:

Operation name: GetEvaluationsQuery

Message: Invariant failed.

Path: ["assetConditionEvaluationRecordsOrError"]

Locations: [{"line":19,"column":3}]

Stack Trace:
  File "/usr/local/lib/python3.10/site-packages/graphql/execution/execute.py", line 521, in execute_field
    result = resolve_fn(source, info, **args)
  File "/usr/local/lib/python3.10/site-packages/dagster_graphql/schema/roots/query.py", line 1151, in resolve_assetConditionEvaluationRecordsOrError
    return fetch_asset_condition_evaluation_records_for_asset_key(
  File "/usr/local/lib/python3.10/site-packages/dagster_graphql/implementation/fetch_asset_condition_evaluations.py", line 109, in fetch_asset_condition_evaluation_records_for_asset_key
    return _get_graphene_records_from_evaluations(
  File "/usr/local/lib/python3.10/site-packages/dagster_graphql/implementation/fetch_asset_condition_evaluations.py", line 58, in _get_graphene_records_from_evaluations
    records=[
  File "/usr/local/lib/python3.10/site-packages/dagster_graphql/implementation/fetch_asset_condition_evaluations.py", line 59, in <listcomp>
    GrapheneAssetConditionEvaluationRecord(
  File "/usr/local/lib/python3.10/site-packages/dagster_graphql/schema/asset_condition_evaluations.py", line 309, in __init__
    evaluation=GrapheneAssetConditionEvaluation(
  File "/usr/local/lib/python3.10/site-packages/dagster_graphql/schema/asset_condition_evaluations.py", line 255, in __init__
    evaluationNodes = [
  File "/usr/local/lib/python3.10/site-packages/dagster_graphql/schema/asset_condition_evaluations.py", line 256, in <listcomp>
    GraphenePartitionedAssetConditionEvaluationNode(evaluation, partitions_def)
  File "/usr/local/lib/python3.10/site-packages/dagster_graphql/schema/asset_condition_evaluations.py", line 161, in __init__
    trueSubset=GrapheneAssetSubset(evaluation.true_subset),
  File "/usr/local/lib/python3.10/site-packages/dagster_graphql/schema/asset_condition_evaluations.py", line 77, in __init__
    subsetValue=GrapheneAssetSubsetValue(asset_subset.subset_value),
  File "/usr/local/lib/python3.10/site-packages/dagster/_core/definitions/asset_subset.py", line 57, in subset_value
    check.invariant(isinstance(self.value, PartitionsSubset))
  File "/usr/local/lib/python3.10/site-packages/dagster/_check/__init__.py", line 1645, in invariant
    raise CheckError("Invariant failed.")

What did you expect to happen?

To work as before

How to reproduce?

No response

Deployment type

None

Deployment details

No response

Additional information

No response

Message from the maintainers

Impacted by this issue? Give it a 👍! We factor engagement into prioritization.

@ion-elgreco ion-elgreco added the type: bug Something isn't working label Jul 1, 2024
@ion-elgreco
Copy link
Contributor Author

Actually this only happens when there are 0 evaluations available for that asset, for assets with an evaluation it works fine

@garethbrickman garethbrickman added the area: auto-materialize Related to Auto Materialization label Jul 1, 2024
@ion-elgreco
Copy link
Contributor Author

Never mind this also happens on assets that should already have evaluations.

@OwenKephart
Copy link
Contributor

Hi @ion-elgreco , do you have a repro for this? Looking into this from our side as well

@ion-elgreco
Copy link
Contributor Author

ion-elgreco commented Jul 3, 2024

@OwenKephart I honestly don't, it's happening across random assets. Whether they are partitioned or non partitioned

Do you have any suggestions how to reproduce?

@OwenKephart
Copy link
Contributor

@ion-elgreco I was actually just able to reproduce this -- looking into a fix.

@ion-elgreco
Copy link
Contributor Author

@ion-elgreco I was actually just able to reproduce this -- looking into a fix.

Awesome!

OwenKephart added a commit that referenced this issue Jul 8, 2024
## 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`GraphenePartitionedAssetConditionEvaluationNode` from 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
`...PartitionedEvaluationNode` from 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: auto-materialize Related to Auto Materialization type: bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants