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] Move cheatsheet to equivalents page #26115

Open
wants to merge 1 commit into
base: dpeng817/airflow-dagster-equivalents
Choose a base branch
from

Conversation

dpeng817
Copy link
Contributor

Summary & Motivation

How I Tested These Changes

Changelog

Insert changelog entry or delete this section.

Comment on lines +590 to +595
Dagster provides rich, searchable{" "}
<a href="/concepts/metadata-tags">
metadata and <a href="/concepts/metadata-tags/tags">tagging</a>
</a>{" "}
support beyond what’s offered by Airflow.
</td>
Copy link

Choose a reason for hiding this comment

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

The nested <a> tags will cause rendering issues in the browser. Consider replacing the text with:

Dagster provides rich, searchable metadata and <a href="/concepts/metadata-tags/tags">tagging</a> support beyond what's offered by Airflow.

Spotted by Graphite Reviewer

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

Comment on lines +626 to +627
I/O managers are more powerful than XComs and allow the passing large
datasets between jobs.
Copy link

Choose a reason for hiding this comment

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

Minor grammatical fix needed: "allow the passing large datasets" should be "allow the passing of large datasets"

Spotted by Graphite Reviewer

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

Comment on lines +522 to +524
Dagster <a href="/concepts/resources">resource</a> contain a superset of
the functionality of hooks and have much stronger composition
guarantees.
Copy link

Choose a reason for hiding this comment

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

Minor grammatical fix needed: "Dagster resource" should be "Dagster resources" since it's referring to the plural concept.

Spotted by Graphite Reviewer

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

@dpeng817 dpeng817 changed the base branch from dpeng817/migration_best_practices to dpeng817/airflow-dagster-equivalents November 24, 2024 04:04
@dpeng817 dpeng817 force-pushed the dpeng817/airflow-dagster-equivalents branch from 2c6f229 to c931603 Compare November 24, 2024 04:37
@dpeng817 dpeng817 force-pushed the dpeng817/airflow-dagster-equivalents branch from c931603 to 10abc83 Compare November 24, 2024 04:40
@dpeng817 dpeng817 force-pushed the dpeng817/airflow-dagster-equivalents branch from 10abc83 to 844a298 Compare November 24, 2024 04:57
@dpeng817 dpeng817 marked this pull request as ready for review November 24, 2024 04:57
@graphite-app graphite-app bot added the docs-to-migrate Docs to migrate to new docs site label Nov 24, 2024
@dpeng817 dpeng817 force-pushed the dpeng817/airflow-dagster-equivalents branch from 844a298 to 2534e40 Compare November 24, 2024 04:59
Copy link

graphite-app bot commented Nov 24, 2024

Graphite Automations

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

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

@dpeng817 dpeng817 force-pushed the dpeng817/airflow-dagster-equivalents branch from 2534e40 to cba5d13 Compare November 24, 2024 18:04
@dpeng817 dpeng817 force-pushed the dpeng817/airflow-dagster-equivalents branch from cba5d13 to 781771a Compare November 24, 2024 18:07
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.

1 participant