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

[AIRFLOW-1083] Fixes the connect error when jaydebeapi >1.0 and the jdbc's autocommit bug #2227

Closed
wants to merge 1 commit into from

Conversation

fordguo
Copy link

@fordguo fordguo commented Apr 7, 2017

Dear Airflow maintainers,

Please accept this PR. I understand that it will not be reviewed until I have checked off all the steps below!

JIRA

- https://issues.apache.org/jira/browse/AIRFLOW-1083
- https://issues.apache.org/jira/browse/AIRFLOW-775

Description

  • I use the latest jaydebeapi 1.1.1, and dont get the correct connection with jdbc_hook:
  • Fixes the AutoCommit in jdbc hook seems not to turn off if set to false [AIRFLOW-775]

@mention-bot
Copy link

@fordguo, thanks for your PR! By analyzing the history of the files in this pull request, we identified @jomar83, @jlowin and @skudriashev to be potential reviewers.

@codecov-io
Copy link

codecov-io commented Apr 7, 2017

Codecov Report

❗ No coverage uploaded for pull request base (master@0da540b). Click here to learn what that means.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #2227   +/-   ##
=========================================
  Coverage          ?   68.69%           
=========================================
  Files             ?      142           
  Lines             ?    10932           
  Branches          ?        0           
=========================================
  Hits              ?     7510           
  Misses            ?     3422           
  Partials          ?        0
Impacted Files Coverage Δ
airflow/hooks/jdbc_hook.py 0% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0da540b...3a05c7b. Read the comment docs.

@r-richmond
Copy link
Contributor

note for maintainers: this is related/a dupe of #2114

@bolkedebruin
Copy link
Contributor

If you want us to consider this please follow to contribution guidelines. Thanks.

@fordguo fordguo changed the title after jaydebeapi >=1.0, use the connect(jclassname, url, driver_args...) [AIRFLOW-1083] Apr 18, 2017
@fordguo fordguo changed the title [AIRFLOW-1083] [AIRFLOW-1083] After jaydebeapi >1.0, use the connect(jclassname, url, driver_args...) Apr 18, 2017
@fordguo
Copy link
Author

fordguo commented Apr 18, 2017

@bolkedebruin @r-richmond Thank for the suggest

@bolkedebruin
Copy link
Contributor

@fordguo can you please follow the commit guidelines? Ie add the JIRA to your commit message subject, squash your commits etc?

then we can merge.

@fordguo fordguo changed the title [AIRFLOW-1083] After jaydebeapi >1.0, use the connect(jclassname, url, driver_args...) [AIRFLOW-1083] Fixes the connect error when jaydebeapi >1.0 and the jdbc's autocommit bug May 13, 2017
@bolkedebruin
Copy link
Contributor

please squash your commits.

@ron819
Copy link
Contributor

ron819 commented Nov 5, 2018

@fordguo are you still working on this?

@fordguo
Copy link
Author

fordguo commented Nov 6, 2018

@fordguo are you still working on this?

@ron819 it worked on my legacy project with old superset which I modified.

@fordguo fordguo closed this Nov 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants