-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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-dlift] project scaffold #25443
base: dpeng817/multi_code_location
Are you sure you want to change the base?
[dagster-dlift] project scaffold #25443
Conversation
803f795
to
4dc5861
Compare
4dc5861
to
830183a
Compare
CLI_REQUIREMENTS = ["click", "structlog"] | ||
setup( | ||
name="dagster-dlift", | ||
version="0.0.26", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The version number in setup.py
(0.0.26
) doesn't match the version specified in CHANGES.md
(0.0.1
) or version.py
(1!0+dev
). To maintain consistency across the project, the version in setup.py
should be changed to 0.0.1
to align with the changelog.
Spotted by Graphite Reviewer
Is this helpful? React 👍 or 👎 to let us know.
author="Dagster Labs", | ||
author_email="[email protected]", | ||
license="Apache-2.0", | ||
description="Tooling to assist with migrating from Airflow to Dagster.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The package description in setup.py
doesn't match the README. The current description refers to Airflow migration tooling, but according to README.md
, this package is for DBT Cloud integration with Dagster. The description should be updated to: "Tooling for observing DBT Cloud instances from within Dagster"
Spotted by Graphite Reviewer
Is this helpful? React 👍 or 👎 to let us know.
"https://github.com/dagster-io/dagster/tree/master/python_modules/libraries/" | ||
"dagster-dlift" | ||
), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The URL path in setup.py
points to python_modules/libraries/dagster-dlift
, but this package is located in examples/experimental/dagster-dlift
. The correct URL should be:
https://github.com/dagster-io/dagster/tree/master/examples/experimental/dagster-dlift
Spotted by Graphite Reviewer
Is this helpful? React 👍 or 👎 to let us know.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The AI kind of cooked with this one
539566d
to
33e00fe
Compare
ed75413
to
ca7b2e7
Compare
33e00fe
to
62b6e25
Compare
@@ -381,6 +381,10 @@ def k8s_extra_cmds(version: str, _) -> List[str]: | |||
"examples/experimental/dagster-airlift/examples/kitchen-sink", | |||
always_run_if=has_dagster_airlift_changes, | |||
), | |||
PackageSpec( | |||
"examples/experimental/dagster-dlift", | |||
name=":dbt: Dbt Cloud-Lift", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this one does not have the same ring to it as airlift ngl
"https://github.com/dagster-io/dagster/tree/master/python_modules/libraries/" | ||
"dagster-dlift" | ||
), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The AI kind of cooked with this one
CLI_REQUIREMENTS = ["click", "structlog"] | ||
setup( | ||
name="dagster-dlift", | ||
version="0.0.26", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
version="0.0.26", | |
version=ver, |
Summary & Motivation
Initial project scaffold for dagster-dlift.
How I Tested These Changes
Added a single test for module load.