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

Count canceled run status as failed in declarative automation #26192

Open
janosroden opened this issue Nov 28, 2024 · 0 comments
Open

Count canceled run status as failed in declarative automation #26192

janosroden opened this issue Nov 28, 2024 · 0 comments
Labels
area: declarative-automation Related to Declarative Automation, AutomationConditions and Auto Materialization type: bug Something isn't working

Comments

@janosroden
Copy link
Contributor

What's the issue?

I'm migrating to AutomationCondition and I noticed that Canceled run status doesn't included to AutomationCondition.execution_failed(). For this reason I have to 'argue' with the AssetDaemon; whenever I cancel a backfill, the canceled runs are queued again immediately. Failed runs are correctly not rescheduled.

I think it would be much more intuitive to include the Canceled status to execution_failed() (like Queued status correctly included to run_in_progress()

If that's not an option, please consider to create AutomationCondition.execution_canceled().

For completeness, let me share my condition:

from dagster import AutomationCondition

partial_automation_cond = (
    # When trigger
    (
        (
            AutomationCondition.missing()
            & AutomationCondition.any_deps_match(
                ~AutomationCondition.missing()
            ).with_label("At least one dep available")
        ).with_label("Can be built from partial deps")
        | AutomationCondition.any_deps_updated()
    ).with_label("Run")
    &
    # When do not trigger
    ~(
        AutomationCondition.in_progress()
        | AutomationCondition.any_deps_in_progress()
        | AutomationCondition.all_deps_match(
            AutomationCondition.execution_failed()
        ).with_label("All deps failed")
        | (
            ~AutomationCondition.any_deps_updated()
            & AutomationCondition.execution_failed()
        ).with_label("Last run failed")
    ).with_label("Skip")
)

What did you expect to happen?

I expect that Canceled runs are considered as a failure. This is a permanent status which is not success, so it makes sense for me.

How to reproduce?

  1. Create A and B assets, B depends on A (sleep in B to have time to cancel)
  2. Use my condition from the description on B
  3. Update A
  4. Wait until B is triggered and cancel
  5. B will be queued again

Dagster version

dagster, version 1.9.3

Deployment type

Local

Deployment details

No response

Additional information

No response

Message from the maintainers

Impacted by this issue? Give it a 👍! We factor engagement into prioritization.
By submitting this issue, you agree to follow Dagster's Code of Conduct.

@janosroden janosroden added the type: bug Something isn't working label Nov 28, 2024
@janosroden janosroden changed the title Consider canceled run status as failed in declarative automation Count canceled run status as failed in declarative automation Nov 29, 2024
@garethbrickman garethbrickman added the area: declarative-automation Related to Declarative Automation, AutomationConditions and Auto Materialization label Dec 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: declarative-automation Related to Declarative Automation, AutomationConditions and Auto Materialization type: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants