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

[dagster-airlift][federation-tutorial] Use declarative automation #25893

Merged
merged 1 commit into from
Nov 14, 2024

Conversation

dpeng817
Copy link
Contributor

Summary & Motivation

How I Tested These Changes

Changelog

Insert changelog entry or delete this section.

@dpeng817 dpeng817 marked this pull request as ready for review November 13, 2024 00:41
@graphite-app graphite-app bot added the docs-to-migrate Docs to migrate to new docs site label Nov 13, 2024
Copy link

graphite-app bot commented Nov 13, 2024

Graphite Automations

"Add a 'docs-to-migrate' label to PRs with docs" took an action on this PR • (11/13/24)

1 label was added to this PR based on Christopher DeCarolis's automation.

@dpeng817 dpeng817 changed the title [dagster-airlift] Use declarative automation [dagster-airlift][federation-tutorial] Use declarative automation Nov 13, 2024
@dpeng817 dpeng817 force-pushed the dpeng817/fed-tutorial-observe-section branch from ea3067a to 2390f28 Compare November 13, 2024 19:00
@dpeng817 dpeng817 force-pushed the dpeng817/fed-tutorial-observe-section branch from 2390f28 to d038556 Compare November 13, 2024 19:10
@dpeng817 dpeng817 force-pushed the dpeng817/fed-tutorial-observe-section branch from d038556 to 3eb8689 Compare November 13, 2024 19:15
@cmpadden cmpadden self-requested a review November 13, 2024 19:19
raise Exception("Dag run failed.")
```

We use the `AirflowInstance` defined earlier to trigger a run of the `customer_metrics` DAG. We then wait for the run to complete, and if it is successful, we'll succcessfully materialize the asset. If the run fails, we'll raise an exception.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's more clear to preface the code with an explanation of what it does, and then follow-up with the code for them to read.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed. fixed

Copy link
Contributor

@cmpadden cmpadden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

couple small points

@dpeng817 dpeng817 force-pushed the dpeng817/fed-tutorial-observe-section branch from 3eb8689 to 9b8354a Compare November 13, 2024 20:01
@dpeng817 dpeng817 force-pushed the dpeng817/fed-tutorial-observe-section branch from 9b8354a to 2adfd07 Compare November 13, 2024 22:00
@dpeng817 dpeng817 force-pushed the dpeng817/fed-tutorial-observe-section branch from 2adfd07 to 71aa819 Compare November 13, 2024 22:03
@dpeng817 dpeng817 force-pushed the dpeng817/executable_and_da branch 3 times, most recently from 6832e0c to 1b7f00f Compare November 13, 2024 22:14
@dpeng817 dpeng817 force-pushed the dpeng817/fed-tutorial-observe-section branch from 71aa819 to 72e4fe5 Compare November 13, 2024 22:18
@dpeng817 dpeng817 force-pushed the dpeng817/fed-tutorial-observe-section branch from 72e4fe5 to 48e9247 Compare November 13, 2024 23:19
@dpeng817 dpeng817 force-pushed the dpeng817/fed-tutorial-observe-section branch from 48e9247 to 86ce701 Compare November 13, 2024 23:47
@dpeng817 dpeng817 force-pushed the dpeng817/fed-tutorial-observe-section branch from 86ce701 to 3940172 Compare November 14, 2024 00:00
@dpeng817 dpeng817 force-pushed the dpeng817/fed-tutorial-observe-section branch from 3940172 to 7da4baa Compare November 14, 2024 18:09
Base automatically changed from dpeng817/fed-tutorial-observe-section to dpeng817/federation-tutorial November 14, 2024 18:10
@dpeng817 dpeng817 merged commit 6a65363 into dpeng817/federation-tutorial Nov 14, 2024
1 of 2 checks passed
@dpeng817 dpeng817 deleted the dpeng817/executable_and_da branch November 14, 2024 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs-to-migrate Docs to migrate to new docs site
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants