Create SQLAlchemy engine from connection in DB Hook and added autocommit param to insert_rows method #40669
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Small request :). PLEASE make sure to have empty line before |
Yes I saw that :) Thanks for fixing that Jarek 👍 |
2 tasks
|
The documentation build failed here: https://github.com/apache/airflow/actions/runs/10004956096/job/27654856973?pr=40669 But when I run it locally with breeze no issues at all. |
|
failed when fatching remote inventory. |
potiuk
reviewed
Jul 19, 2024
potiuk
approved these changes
Jul 26, 2024
… the get_conn result to build the sqlalchemy engine
…e run method as this parameter will also be needed once whe have the SQLInsertRowsOperator
… has no attribute 'get_bind'
…ements the sqlalchemy_url property then use it, otherwise fallback to original implementation with get_uri
…chemy_scheme parameter
potiuk
force-pushed
the
feature/create-sql-alchemy-engine-from-db-conn
branch
from
14a6489
to
bef02a3
Compare
|
Rebased to see if it still works in main. |
Thank you @potiuk, cannot check it myself right now on vacation 😉 |
potiuk
approved these changes
Jul 26, 2024
utkarsharma2
added
the
changelog:skip
molcay
pushed a commit
to VladaZakharova/airflow
that referenced
this pull request
Aug 19, 2024
…mit param to insert_rows method (apache#40669) * refactor: Refactored get_sqlalchemy_engine method of DbApiHook to use the get_conn result to build the sqlalchemy engine * refactor: Added autocommit parameter to insert_rows just like with the run method as this parameter will also be needed once whe have the SQLInsertRowsOperator * refactor: Updated the docstring of the insert_rows method * refactor: Updated sql.pyi * refactor: Try to fix AttributeError: type object 'SkipDBTestsSession' has no attribute 'get_bind' * refactor: Implemented the sqlalchemy_url property for JdbcHook * refactor: Refactored get_sqlalchemy_engine in DbApiHook, if Hook implements the sqlalchemy_url property then use it, otherwise fallback to original implementation with get_uri * refactor: Added SQLAlchemy Inspector property in DbApiHook * refactor: Reformated test_sqlalchemy_url_with_sqlalchemy_scheme in TestJdbcHook * refactor: Fixed static checks in DbApiHook * refactor: Fixed some static checks * docs: Updated docstring of JdbcHook and mentioned importance of sqlalchemy_scheme parameter --------- Co-authored-by: David Blain <david.blain@infrabel.be>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Refactored the get_sqlalchemy_engine method in DbAPiHook so it's using the uri as well as the connection for creating an engine, which in most cases nowadays isn't working for a lot of database dialects anyway and which also bypasses the know issues with the get_uri method in combination with sqlalchemy that will be fixed in #39572.
Beside that this PR also added the autocommit parameter to the insert_rows method of the DbAPiHook, just like it is already the case with the run method. This is a preparation for a new PR that will introduce an SQLInsertRowsOperator, which will be a convenient operator allowing you to insert rows using an operator instead of the need to use a PythonOperator in combination with the already existing insert_rows method of the DbApiHook, that way it will be directly possible to pass an XCom containing the rows to persist to the operator which aren't directly feasible by using the GenericTransfer operator, for example in the case of have to download data from a REST endpoint and persisting it directly to a database.
^ 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.