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

AutomationCondition API migration guide #26223

Closed
LPauzies opened this issue Dec 2, 2024 · 5 comments · Fixed by #26390
Closed

AutomationCondition API migration guide #26223

LPauzies opened this issue Dec 2, 2024 · 5 comments · Fixed by #26390
Labels
area: declarative-automation Related to Declarative Automation, AutomationConditions and Auto Materialization area: docs Related to documentation in general area: sensor Related to Sensors

Comments

@LPauzies
Copy link

LPauzies commented Dec 2, 2024

What's the issue?

I'm trying to migrate sensors from Dagster 1.7.4 to 1.9.2.
As asked by the framework some entities are deprecated, and it is the case of @multi_asset_sensor, saying that we should use AutomationConditions instead :

@deprecated(breaking_version="2.0.0", additional_warn_text="use `AutomationConditions` instead")

I don't find the documentation very clear and there is no migration guide (I think it should be a migration guide as the way we implement things change totally).

I would want to migrate the following code to the one using AutomationConditions API :

from dagster import AssetKey, RunRequest, SensorEvaluationContext, multi_asset_sensor

from jobs import job_4, job_5

@multi_asset_sensor(
    name="dbt_and_native_implemented_assets_sensor",
    description="A sensor that refreshes DBT stuff and materialize given sensors.",
    monitored_assets=[
        AssetKey(path=["code_loc_1", "dbt", "asset_1_job_1"]),
        AssetKey(path=["code_loc_1", "dbt", "asset_2_job_1"]),
        AssetKey(path=["code_loc_1", "dbt", "asset_3_job_1"]),
        AssetKey(path=["code_loc_2", "python", "asset_1_job_2"]),
        AssetKey(path=["code_loc_2", "python", "asset_2_job_2"]),
        AssetKey(path=["code_loc_2", "python", "asset_3_job_2"]),
        AssetKey(path=["code_loc_2", "python", "asset_4_job_2"]),
        AssetKey(path=["code_loc_3", "dbt", "asset_1_job_3"]),
        AssetKey(path=["code_loc_3", "python", "asset_2_job_3"]),
    ],
    jobs=[
        job_4,
        job_5,
    ],
)
def dbt_and_native_implemented_assets_sensor(context: SensorEvaluationContext):
    asset_events = context.latest_materialization_records_by_key()
    if all(asset_events.values()):
        context.advance_all_cursors()
        yield RunRequest(job_name="job_4")
        yield RunRequest(job_name="job_5")

The documentation says here : https://docs.dagster.io/concepts/automation#selecting-a-method, that in the case above, we should choose https://docs.dagster.io/concepts/partitions-schedules-sensors/asset-sensors but the deprecation decorator shows that we should use AutomationConditions). Who is right and how to handle my use case if I have to use AutomationCondition ? How would you do ?

What did you expect to happen?

I expect the documentation to be clearer with some examples showing how to migrate from @multi_asset_sensor to AutomationConditions API. Maybe with some example with DBT for example and how to work with job triggering using AutomationConditions if the deprecation message is true. Otherwise specify that we should use assets_sensors as above.

How to reproduce?

No response

Dagster version

dagster, version 1.9.2

Deployment type

Other Docker-based deployment

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.

@LPauzies LPauzies added the type: bug Something isn't working label Dec 2, 2024
@garethbrickman garethbrickman added area: docs Related to documentation in general area: sensor Related to Sensors area: declarative-automation Related to Declarative Automation, AutomationConditions and Auto Materialization and removed type: bug Something isn't working labels Dec 2, 2024
@lydialimlh
Copy link

I have the same question. I was previously using @multi_asset_sensor because the regular sensors weren't able to sense specific partitions and pass them on to the run requests. Maybe there is a better way now but I'm also puzzled by the recommendation to use AutomationConditions

@smackesey
Copy link
Collaborator

Thanks for reporting this. We've realized that, while automation conditions are a good replacement for the most common use case of @multi_asset_sensor in the past (triggering materializations of a target asset when upstream assets were materialized), automation conditions don't cover all use cases-- for example, when launching jobs from the sensor, as in OP here. So we are going to roll back deprecation of @multi_asset_sensor as a whole and may emit deprecation warnings in more narrowly scoped circumstances.

There is a migration guide from auto-materialize policies to automation conditions, but it unfortunately doesn't cover multi-asset sensors.

@LPauzies
Copy link
Author

Thanks for the the answer and the reactivity 🙏

smackesey added a commit that referenced this issue Dec 19, 2024
Resolves #26223 

## Summary & Motivation

`@multi_asset_sensor` was deprecated with a message saying to use
`AutomationCondition`, but `AutomationCondition` cannot cover all of
`@multi_asset_sensor` uses cases (like running jobs), as discussed in
#26223.

## Changelog

Deprecation of `@multi_asset_sensor` has been rolled back.
@sugendran
Copy link

I just spent the best part of a day trying to work out what I was missing, and this was it. Could you update the docs to explain that this is not supported? Or at least the bot?

Also, is there something in the works that will have the declarative automation system support this?

@smackesey
Copy link
Collaborator

I just spent the best part of a day trying to work out what I was missing, and this was it. Could you update the docs to explain that this is not supported? Or at least the bot?

Can you clarify? This issue was about the deprecation of multi-asset sensors, but we reversed their deprecation.

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 area: docs Related to documentation in general area: sensor Related to Sensors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants