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

[INTEGRATION][Dagster] Handle updated PipelineRun in OpenLineage sensor unit test #624

Conversation

dominiquetipton
Copy link
Contributor

Problem

In Dagster 0.14.4, pipeline_name was changed from an optional argument to a required argument of PipelineRun per dagster-io/dagster#6917. This is Dagster's internal change that should not affect the functionality of the OpenLineage sensor, but the unit test helper function that creates a PipelineRun needs an update to include the required argument.

Solution

Update unit test helper method to include the required pipeline_name argument in PipelineRun.

Checklist

  • You've signed-off your work
  • Your pull request title follows our guidelines
  • Your changes are accompanied by tests (if relevant)
  • Your change contains a small diff and is self-contained
  • You've updated any relevant documentation (if relevant)
  • You've updated the CHANGELOG.md with details about your change under the "Unreleased" section (if relevant, depending on the change, this may not be necessary)
  • You've versioned the core OpenLineage model or facets according to SchemaVer (if relevant)

In Dagster 0.14.4, `pipeline_name` was changed from an optional argument to a required argument of PipelineRun. This is Dagster's internal change that should not affect the functionality of the OpenLineage sensor, but the unit test helper function that creates a PipelineRun needs an update to include the required argument.

Signed-off-by: Dominique Tipton <dominiquetipton@northwesternmutual.com>
@codecov-commenter
Copy link

Codecov Report

Merging #624 (49991d2) into main (e0dd371) will increase coverage by 6.76%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #624      +/-   ##
==========================================
+ Coverage   83.98%   90.75%   +6.76%     
==========================================
  Files          36        6      -30     
  Lines        2142      238    -1904     
==========================================
- Hits         1799      216    -1583     
+ Misses        343       22     -321     
Impacted Files Coverage Δ
integration/common/openlineage/common/utils.py
...on/airflow/openlineage/lineage_backend/__init__.py
integration/common/openlineage/common/dataset.py
...ion/airflow/openlineage/airflow/extractors/base.py
...ntegration/airflow/openlineage/airflow/__init__.py
...airflow/extractors/great_expectations_extractor.py
...flow/openlineage/airflow/extractors/example_dag.py
integration/common/openlineage/common/models.py
integration/airflow/openlineage/airflow/adapter.py
...neage/common/provider/great_expectations/facets.py
... and 20 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@mobuchowski mobuchowski self-requested a review March 22, 2022 21:57
@mobuchowski mobuchowski merged commit 21b039b into OpenLineage:main Mar 23, 2022
@dominiquetipton dominiquetipton deleted the dagster/openlineage-sensor-update-tests branch March 23, 2022 15:39
wjohnson pushed a commit to wjohnson/OpenLineage that referenced this pull request Mar 24, 2022
)

In Dagster 0.14.4, `pipeline_name` was changed from an optional argument to a required argument of PipelineRun. This is Dagster's internal change that should not affect the functionality of the OpenLineage sensor, but the unit test helper function that creates a PipelineRun needs an update to include the required argument.

Signed-off-by: Dominique Tipton <dominiquetipton@northwesternmutual.com>
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.

3 participants