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][rfc] Multi code locations working #25343

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dpeng817
Copy link
Contributor

@dpeng817 dpeng817 commented Oct 17, 2024

Summary & Motivation

Working example of splitting an airflow instance into multiple code locations.

The main interesting bit here is the surface area we expose to filter down the dags. I think it makes sense, but my question is; do we envision needing to split further? IE, what if you have a really complicated dag where you want to split individual tasks out? We might need another pluggability layer for that.

How I Tested These Changes

Added a new kitchen sink full example of splitting an instance into two code locations.

Changelog

NOCHANGELOG

Copy link
Contributor Author

dpeng817 commented Oct 17, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @dpeng817 and the rest of your teammates on Graphite Graphite

@dpeng817 dpeng817 marked this pull request as ready for review October 17, 2024 17:16
@dpeng817 dpeng817 changed the title [dagster-airlift] Multi code locations working [dagster-airlift][rfc] Multi code locations working Oct 17, 2024
Copy link
Member

@schrockn schrockn left a comment

Choose a reason for hiding this comment

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

This is pretty great. I thought this would be harder.

@dpeng817 dpeng817 force-pushed the dpeng817/proxy_partitioning_working branch from e20b6b8 to 2c055e5 Compare October 18, 2024 17:34
ExpectedMat(AssetKey("dag_first_code_location__asset"), runs_in_dagster=False)
],
"dag_second_code_location": [
ExpectedMat(AssetKey("dag_first_code_location__asset"), runs_in_dagster=False)
Copy link

Choose a reason for hiding this comment

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

The asset key for the second DAG appears to be incorrect. It should be AssetKey("dag_second_code_location__asset") instead of AssetKey("dag_first_code_location__asset"). This change will ensure that the asset key correctly corresponds to the DAG it represents.

Spotted by Graphite Reviewer

Is this helpful? React 👍 or 👎 to let us know.

@dpeng817 dpeng817 force-pushed the dpeng817/proxy_partitioning_working branch from 2c055e5 to 5d3e709 Compare October 18, 2024 18:26
@dpeng817 dpeng817 force-pushed the dpeng817/proxy_partitioning_working branch from 86a320e to af598c3 Compare October 21, 2024 17:15
@dpeng817 dpeng817 force-pushed the dpeng817/proxy_partitioning_working branch from af598c3 to 510cb79 Compare October 22, 2024 17:07
@dpeng817 dpeng817 force-pushed the dpeng817/proxy_partitioning_working branch from 510cb79 to 902ae76 Compare October 23, 2024 19:56
Base automatically changed from dpeng817/proxy_partitioning_working to master October 23, 2024 21:39
@dpeng817 dpeng817 force-pushed the dpeng817/multi_code_location branch 2 times, most recently from 315cf37 to ed75413 Compare October 24, 2024 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants