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

Python driver implements DB-API. #8

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

ipr0
Copy link
Collaborator

@ipr0 ipr0 commented Apr 13, 2020

Run tests:
bazel test //driver/python:all_tests --test_output=streamed --runs_per_test=2

@ipr0 ipr0 requested a review from szab100 April 13, 2020 01:18
@ipr0 ipr0 force-pushed the python branch 3 times, most recently from 52f5aab to 1352fec Compare April 13, 2020 01:26
@ipr0 ipr0 linked an issue Apr 13, 2020 that may be closed by this pull request
Copy link
Owner

@szab100 szab100 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, thank you, this is truly awesome work Ievgen!!!
Left just a few nits, no rush, really.. Thank you!

driver/python/BUILD Outdated Show resolved Hide resolved
driver/python/BUILD Outdated Show resolved Hide resolved
driver/python/connection.py Outdated Show resolved Hide resolved
driver/python/connection.py Outdated Show resolved Hide resolved
self.rowcount = self._set_data(descriptors, resp.rows)
else:
self.rowcount = 0
return True
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this loop's structure could be refactored a bit or at least the order of the two resp.status == if statements could be flipped (and maybe use else-if for the 2nd), so it is better visible that we only retry if it's a REDIRECT

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

with urlopen(self.health_check_url) as resp:
if HTTPStatus.OK == resp.getcode():
return True
except URLError:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in which case(s) can we get an URLError here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Connection or internal DB errors.

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.

Implement a Python standard DB-API driver
2 participants