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

feat: Add OpenLineage support for CloudSQLExecuteQueryOperator #45182

Merged

Conversation

kacpermuda
Copy link
Contributor

@kacpermuda kacpermuda commented Dec 23, 2024

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.rst or {issue_number}.significant.rst, in newsfragments.

@boring-cyborg boring-cyborg bot added area:providers provider:google Google (including GCP) related issues labels Dec 23, 2024
@kacpermuda kacpermuda force-pushed the feat-ol-google-cloud-sql-operator branch 3 times, most recently from 74aa67e to 2782f2f Compare December 24, 2024 10:05
@kacpermuda kacpermuda changed the title feat: Add OpenLineage support for CloudSQLExecuteQueryOperator feat: Add OpenLineage support for CloudSQLExecuteQueryOperator and some SQLtoGCS transfer operators Dec 24, 2024
@kacpermuda kacpermuda force-pushed the feat-ol-google-cloud-sql-operator branch from 2782f2f to e26d1b3 Compare December 27, 2024 11:10
@kacpermuda kacpermuda changed the title feat: Add OpenLineage support for CloudSQLExecuteQueryOperator and some SQLtoGCS transfer operators feat: Add OpenLineage support for CloudSQLExecuteQueryOperator Dec 27, 2024
@kacpermuda kacpermuda marked this pull request as ready for review December 27, 2024 11:37
@kacpermuda kacpermuda force-pushed the feat-ol-google-cloud-sql-operator branch from e26d1b3 to b2b65a3 Compare December 27, 2024 14:58
@kacpermuda kacpermuda force-pushed the feat-ol-google-cloud-sql-operator branch 7 times, most recently from ae50afe to ea03966 Compare January 3, 2025 12:03
@kacpermuda kacpermuda marked this pull request as draft January 3, 2025 12:03
@kacpermuda kacpermuda force-pushed the feat-ol-google-cloud-sql-operator branch 3 times, most recently from fb64caa to 9246d0c Compare January 3, 2025 16:08
@kacpermuda kacpermuda marked this pull request as ready for review January 3, 2025 16:49
@kacpermuda kacpermuda force-pushed the feat-ol-google-cloud-sql-operator branch 2 times, most recently from 4fde5b4 to 926b46e Compare January 6, 2025 11:19
@kacpermuda
Copy link
Contributor Author

@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

@michalmodras
Copy link

@fdemiane is currently working on some fixes in the Google system tests dashboard; @fdemiane can you please help @kacpermuda ?

@mobuchowski
Copy link
Contributor

@kacpermuda should we draft this PR until you verify if it works?

@kacpermuda kacpermuda marked this pull request as draft January 8, 2025 18:37
@kacpermuda
Copy link
Contributor Author

Yeah, we can do that. I've tested it manually but if we can run the system tests as well it would be better.

@fdemiane
Copy link
Contributor

Hello @kacpermuda,
Indeed the Google Providers Dashboard is down for maintenance. The example_cloud_sql_query.py system test was also failing on main since a long time, and I wasn't able to run it either. This needs investigation and most probably changes to the system test code.
In the light of the above, I can recommend sticking to manually testing the operator, or maybe try to manually replicate in some way what the system test is doing for greater coverage

@kacpermuda kacpermuda force-pushed the feat-ol-google-cloud-sql-operator branch from 926b46e to b058b65 Compare January 15, 2025 13:16
@kacpermuda kacpermuda marked this pull request as ready for review January 21, 2025 09:42
@kacpermuda
Copy link
Contributor Author

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.

Copy link
Contributor

@mobuchowski mobuchowski left a 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

@mobuchowski mobuchowski merged commit 40590ed into apache:main Jan 22, 2025
61 checks passed
@kacpermuda kacpermuda deleted the feat-ol-google-cloud-sql-operator branch January 23, 2025 07:37
dauinh pushed a commit to dauinh/airflow that referenced this pull request Jan 24, 2025
@utkarsharma2 utkarsharma2 added the changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) label Jan 27, 2025
gpathak128 pushed a commit to gpathak128/airflow that referenced this pull request Jan 29, 2025
got686-yandex pushed a commit to got686-yandex/airflow that referenced this pull request Jan 30, 2025
niklasr22 pushed a commit to niklasr22/airflow that referenced this pull request Feb 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:providers changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) provider:google Google (including GCP) related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants