-
Notifications
You must be signed in to change notification settings - Fork 16.4k
feat: Add OpenLineage support for CloudSQLExecuteQueryOperator #45182
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: Add OpenLineage support for CloudSQLExecuteQueryOperator #45182
Conversation
74aa67e to
2782f2f
Compare
2782f2f to
e26d1b3
Compare
e26d1b3 to
b2b65a3
Compare
ae50afe to
ea03966
Compare
fb64caa to
9246d0c
Compare
4fde5b4 to
926b46e
Compare
|
@ahidalgob @michalmodras As an addition to my manual tests I've done I wanted to run system tests for CloudSQLExecuteQueryOperator, but they seem to fail even on current main (breeze testing system-tests providers/tests/system/google/cloud/cloud_sql/example_cloud_sql_query.py). Also, the system test dashboard provided by Google is not available, so I can't verify if those system tests fails only for me. Would appreciate any help in running those tests |
|
@fdemiane is currently working on some fixes in the Google system tests dashboard; @fdemiane can you please help @kacpermuda ? |
|
@kacpermuda should we draft this PR until you verify if it works? |
|
Yeah, we can do that. I've tested it manually but if we can run the system tests as well it would be better. |
|
Hello @kacpermuda, |
Signed-off-by: Kacper Muda <mudakacper@gmail.com>
926b46e to
b058b65
Compare
|
Thanks for the information. I've run some more manual tests, I think I even managed to run a system test with proxy (that was my main concern, if the proxy still works fine). I think it can be merged now, and once the dashboard is live again I'll check again if all works well. |
mobuchowski
left a comment
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.
Approved with one question
…e#45182) Signed-off-by: Kacper Muda <mudakacper@gmail.com>
…e#45182) Signed-off-by: Kacper Muda <mudakacper@gmail.com>
…e#45182) Signed-off-by: Kacper Muda <mudakacper@gmail.com>
…e#45182) Signed-off-by: Kacper Muda <mudakacper@gmail.com>
This PR adds OpenLineage support for CloudSQLExecuteQueryOperator. The logic is very similar (almost the same) as in SQLExecuteQueryOperator where we take advantage of DB hooks being instrumented.
Adding OL required changing sql proxy in the operator to a context manager, probably a nit change.
I've run some manual tests and all looks good.
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rstor{issue_number}.significant.rst, in newsfragments.