update openlineage to avoid deprecation warning #1449
Conversation
openlineage suggests using event_v2 as the new import call
UserWarning: {message : DeprecationWarning('This module is deprecated. Please use `openlineage.client.event_v2`.'), category : 'DeprecationWarning', filename : '/home/airflow/.local/lib/python3.10/site-packages/openlineage/client/filter.py', lineno : 9, line : None}
dosubot
bot
added
the
size:XS
✅ Deploy Preview for sunny-pastelito-5ecb04 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
dosubot
bot
added
the
area:lineage
cosmos/operators/local.py
Outdated
| @@ -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 | |||
There was a problem hiding this comment.
If we are planning to do this, we may have to upgrade open lineage dependency
There was a problem hiding this comment.
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
There was a problem hiding this comment.
yes, +1 to something like
try:
from openlineage.client.event_v2 import RunEvent
except ImportError:
from openlineage.client.run import RunEvent
There was a problem hiding this comment.
@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
There was a problem hiding this comment.
thanks @CorsettiS . Can we then add # pragma: no cover to ignore those lines from test coverage so that the CI gets green?
There was a problem hiding this comment.
This is really helpful, @CorsettiS , thank you very much for pointing this out and submitting a fix. I left some minor feedback in-line
cosmos/operators/local.py
Outdated
| @@ -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 | |||
There was a problem hiding this comment.
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
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1449 +/- ##
==========================================
- Coverage 96.94% 96.87% -0.07%
==========================================
Files 73 73
Lines 4355 4358 +3
==========================================
Hits 4222 4222
- Misses 133 136 +3 ☔ View full report in Codecov by Sentry. |
Description
openlineage suggests using event_v2 as the new import call. This can be seen in the source code of openlineage itself for the run method.
Related Issue(s)
UserWarning: {message : DeprecationWarning('This module is deprecated. Please use
openlineage.client.event_v2.'), category : 'DeprecationWarning', filename : '/home/airflow/.local/lib/python3.10/site-packages/openlineage/client/filter.py', lineno : 9, line : None}Breaking Change?
No
Checklist