-
Notifications
You must be signed in to change notification settings - Fork 14.2k
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
[AIRFLOW-5582] AutoCommit in jdbc is missing get_autocommit #6232
Conversation
Codecov Report
@@ Coverage Diff @@
## master #6232 +/- ##
==========================================
+ Coverage 80.05% 83.31% +3.25%
==========================================
Files 610 645 +35
Lines 35264 37293 +2029
==========================================
+ Hits 28232 31071 +2839
+ Misses 7032 6222 -810
Continue to review full report at Codecov.
|
558f9f1
to
d719e1f
Compare
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.
Welcome @blindlf ! :)
airflow/hooks/jdbc_hook.py
Outdated
Return True if conn.autocommit is set to True. | ||
Return False if conn.autocommit is not set or set to False | ||
|
||
:param conn: The connection |
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.
Could you also add the type, please? Either via the docs :type
or in the function header directly to the arg conn:
@blindlf any progress with addressing the comments? |
Sorry for the late. Comments are done, I'm trying to work on unit test. |
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.
You can assert that a mock object has been called exactly once.
I'm not sure about this. |
Because that's what the function should do if you call it once. |
…ed exactly once Co-Authored-By: Felix Uellendall <[email protected]>
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.
Thank you for your patience @blindlf :)
- add tests - update docs Co-Authored-By: Felix Uellendall <[email protected]> (cherry picked from commit d03fb28)
- add tests - update docs Co-Authored-By: Felix Uellendall <[email protected]> (cherry picked from commit d03fb28)
- add tests - update docs Co-Authored-By: Felix Uellendall <[email protected]> (cherry picked from commit d03fb28)
Make sure you have checked all steps below.
Jira
Description
JdbcHook.set_autocommit update by conn.jconn.setAutoCommit, DbApiHook.get_autocommit retrieve by conn.autocommit.
After DbApiHook.execute(sql), conn.commit() will throws exception.
Tests
Commits