-
Notifications
You must be signed in to change notification settings - Fork 3k
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
feat(ingest/dagster): Dagster source #10071
Conversation
cache: "pip" | ||
- name: Install dependencies | ||
run: ./metadata-ingestion/scripts/install_deps.sh | ||
- name: Install dagster package and test (extras ${{ matrix.extraPythonRequirement }}) |
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.
[actionlint] reported by reviewdog 🐶
property "extrapythonrequirement" is not defined in object type {python-version: number} [expression]
- name: Install dependencies | ||
run: ./metadata-ingestion/scripts/install_deps.sh | ||
- name: Install dagster package and test (extras ${{ matrix.extraPythonRequirement }}) | ||
run: ./gradlew -Pextra_pip_requirements='${{ matrix.extraPythonRequirement }}' :metadata-ingestion-modules:dagster-plugin:lint :metadata-ingestion-modules:dagster-plugin:testQuick |
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.
[actionlint] reported by reviewdog 🐶
property "extrapythonrequirement" is not defined in object type {python-version: number} [expression]
if: always() | ||
run: source metadata-ingestion-modules/dagster-plugin/venv/bin/activate && pip freeze | ||
- uses: actions/upload-artifact@v3 | ||
if: ${{ always() && matrix.python-version == '3.10' && matrix.extraPythonRequirement == 'dagster>=1.3.3' }} |
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.
[actionlint] reported by reviewdog 🐶
property "extrapythonrequirement" is not defined in object type {python-version: number} [expression]
token: ${{ secrets.CODECOV_TOKEN }} | ||
directory: . | ||
fail_ci_if_error: false | ||
flags: dagster-${{ matrix.python-version }}-${{ matrix.extraPythonRequirement }} |
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.
[actionlint] reported by reviewdog 🐶
property "extrapythonrequirement" is not defined in object type {python-version: number} [expression]
...ngestion-modules/dagster-plugin/src/datahub_dagster_plugin/example_jobs/advanced_ops_jobs.py
Outdated
Show resolved
Hide resolved
...ngestion-modules/dagster-plugin/src/datahub_dagster_plugin/example_jobs/advanced_ops_jobs.py
Outdated
Show resolved
Hide resolved
reverting path_spec change
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.
small comments, but LGTM!
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.
are we actually using tox?
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.
do the example jobs need to be packaged into the datahub_dagster_plugin code?
we did it for airflow because they autogenerated docs based on them, but I think a more standard dagster-plugin/examples directory would be better here
- Removing tox.ini
Co-authored-by: shubhamjagtap639 <[email protected]>
Checklist