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] [api-docs] in-airflow section #25692

Merged
merged 1 commit into from
Nov 7, 2024

Conversation

dpeng817
Copy link
Contributor

@dpeng817 dpeng817 commented Nov 1, 2024

Summary & Motivation

Scaffold out in-airflow section, docstrings for all the operators and the main proxying_to_dagster function.

How I Tested These Changes

Eyes

Copy link

github-actions bot commented Nov 1, 2024

Deploy preview for dagster-docs ready!

Preview available at https://dagster-docs-phnjypi9l-elementl.vercel.app
https://dpeng817-docs-in-airflow.dagster.dagster-docs.io

Direct link to changed pages:

@dpeng817 dpeng817 force-pushed the dpeng817/docs/mapping branch from b550d3b to 8a5142b Compare November 1, 2024 22:01
@dpeng817 dpeng817 force-pushed the dpeng817/docs/in-airflow branch from c7ddb77 to 8f2e0c3 Compare November 1, 2024 22:01
@dpeng817 dpeng817 force-pushed the dpeng817/docs/mapping branch from 8a5142b to 729fc69 Compare November 1, 2024 22:09
@dpeng817 dpeng817 force-pushed the dpeng817/docs/in-airflow branch from 8f2e0c3 to e3208b2 Compare November 1, 2024 22:09
proxying_to_dagster(
proxied_state=load_proxied_state_from_yaml(Path(__file__).parent / "proxied_state"),
global_vars=globals(),
build_from_dag_fn=custom_build_from_dag_fn,
Copy link

Choose a reason for hiding this comment

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

The example references custom_build_from_dag_fn, but this variable isn't defined. Based on the example code above, this should be CustomAuthDAGProxyOperator.build_from_dag to match the class method implementation.

Spotted by Graphite Reviewer

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

@dpeng817 dpeng817 force-pushed the dpeng817/docs/in-airflow branch 2 times, most recently from a700b73 to 796ef48 Compare November 6, 2024 22:21
@dpeng817 dpeng817 force-pushed the dpeng817/docs/mapping branch from ef76b25 to d66d4d1 Compare November 6, 2024 22:22
@dpeng817 dpeng817 force-pushed the dpeng817/docs/in-airflow branch from 796ef48 to 614ea46 Compare November 6, 2024 22:22
Copy link

github-actions bot commented Nov 6, 2024

Deploy preview for dagster-docs-beta ready!

Preview available at https://dagster-docs-beta-ewnyotw10-elementl.vercel.app

Direct link to changed pages:

@dpeng817 dpeng817 force-pushed the dpeng817/docs/in-airflow branch from 614ea46 to 80bc0f1 Compare November 6, 2024 23:23
Comment on lines +96 to +99
if task.task_id == "my_task_needs_more_retries":
return CustomAuthTaskProxyOperator(task_id=task_id, retries=3)
else:
return CustomAuthTaskProxyOperator(task_id=task_id)
Copy link

Choose a reason for hiding this comment

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

The variable task_id in this example is undefined - it should reference the task's ID via task.task_id. Here's the corrected code:

if task.task_id == "my_task_needs_more_retries":
    return CustomAuthTaskProxyOperator(task_id=task.task_id, retries=3)
else:
    return CustomAuthTaskProxyOperator(task_id=task.task_id)

Spotted by Graphite Reviewer

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

@cmpadden cmpadden self-requested a review November 7, 2024 17:21
@dpeng817 dpeng817 force-pushed the dpeng817/docs/mapping branch from d66d4d1 to 85675c1 Compare November 7, 2024 19:47
@dpeng817 dpeng817 force-pushed the dpeng817/docs/in-airflow branch from 80bc0f1 to 51abea2 Compare November 7, 2024 19:47
@dpeng817 dpeng817 force-pushed the dpeng817/docs/mapping branch from 85675c1 to 25a1908 Compare November 7, 2024 19:52
@dpeng817 dpeng817 force-pushed the dpeng817/docs/in-airflow branch from 51abea2 to 5b40255 Compare November 7, 2024 19:52
@dpeng817 dpeng817 force-pushed the dpeng817/docs/mapping branch from 25a1908 to 3e41d65 Compare November 7, 2024 19:54
@dpeng817 dpeng817 force-pushed the dpeng817/docs/in-airflow branch from 5b40255 to 0b65288 Compare November 7, 2024 19:54
@dpeng817 dpeng817 force-pushed the dpeng817/docs/mapping branch from 3e41d65 to c74f9e3 Compare November 7, 2024 21:17
@dpeng817 dpeng817 force-pushed the dpeng817/docs/in-airflow branch from 0b65288 to e8d4b33 Compare November 7, 2024 21:17
@dpeng817 dpeng817 force-pushed the dpeng817/docs/mapping branch from c74f9e3 to 8317171 Compare November 7, 2024 21:21
@dpeng817 dpeng817 force-pushed the dpeng817/docs/in-airflow branch from e8d4b33 to 59b72b2 Compare November 7, 2024 21:22
@dpeng817 dpeng817 force-pushed the dpeng817/docs/mapping branch from 8317171 to 1cb706e Compare November 7, 2024 21:29
@dpeng817 dpeng817 force-pushed the dpeng817/docs/in-airflow branch from 59b72b2 to 4bac474 Compare November 7, 2024 21:29
@dpeng817 dpeng817 force-pushed the dpeng817/docs/mapping branch from 1cb706e to d78ecda Compare November 7, 2024 21:51
@dpeng817 dpeng817 force-pushed the dpeng817/docs/in-airflow branch from 4bac474 to 036db8b Compare November 7, 2024 21:51
@dpeng817 dpeng817 force-pushed the dpeng817/docs/mapping branch from d78ecda to 6ca006a Compare November 7, 2024 22:06
@dpeng817 dpeng817 force-pushed the dpeng817/docs/in-airflow branch from 036db8b to ff64f05 Compare November 7, 2024 22:07
Base automatically changed from dpeng817/docs/mapping to master November 7, 2024 22:27
@dpeng817 dpeng817 force-pushed the dpeng817/docs/in-airflow branch from ff64f05 to b841062 Compare November 7, 2024 22:29
@dpeng817 dpeng817 merged commit 6c3f966 into master Nov 7, 2024
2 of 3 checks passed
@dpeng817 dpeng817 deleted the dpeng817/docs/in-airflow branch November 7, 2024 22:31
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