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.

Sign up for GitHub

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

update openlineage to avoid deprecation warning #1449

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

update openlineage to avoid deprecation warning #1449

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9ef0af1
update openlineage to avoid deprecation warning
CorsettiS Jan 13, 2025
6b09131
Update local.py
CorsettiS Jan 14, 2025
75d0f45
🎨 [pre-commit.ci] Auto format from pre-commit.com hooks
pre-commit-ci[bot] Jan 14, 2025
8f4d2bd
Merge branch 'main' into patch-1
CorsettiS Jan 14, 2025
ff8deaf
Update local.py
CorsettiS Jan 17, 2025
dbf3eca
Merge branch 'main' into patch-1
CorsettiS Jan 17, 2025
bc947ae
🎨 [pre-commit.ci] Auto format from pre-commit.com hooks
pre-commit-ci[bot] Jan 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cosmos/operators/local.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
if TYPE_CHECKING:
from airflow.datasets import Dataset # noqa: F811
from dbt.cli.main import dbtRunner, dbtRunnerResult
from openlineage.client.run import RunEvent
from openlineage.client.event_v2 import RunEvent
Copy link
Contributor

Choose a reason for hiding this comment

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

If we are planning to do this, we may have to upgrade open lineage dependency

Copy link
Collaborator

Choose a reason for hiding this comment

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

It is important we support the old version of the OL package and the new, perhaps using a try/catch exception statement.

Also, if possible, we'd add some test. Perhaps we could have a subset of the tests using an older version of OL, eg.:
https://github.com/astronomer/astronomer-cosmos/blob/main/.github/workflows/test.yml#L270

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, +1 to something like

try:
    from openlineage.client.event_v2 import RunEvent
except ImportError:
    from openlineage.client.run import RunEvent

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pankajkoti fair enough, I have added the try-except condition. It seems like a very small case to add subsets of tests on top of it IMO tho

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks @CorsettiS . Can we then add # pragma: no cover to ignore those lines from test coverage so that the CI gets green?



from sqlalchemy.orm import Session
Expand Down
Loading