-
Notifications
You must be signed in to change notification settings - Fork 28
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: Implementing DB-API types according to the PEP-0249 specification #521
Conversation
google/cloud/spanner_dbapi/types.py
Outdated
This object describes (long) binary columns in a database (e.g. LONG, RAW, BLOBS). | ||
""" | ||
def _time_from_ticks(ticks, tz=None): | ||
"""A helper method used to construct a DB-API time value. |
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.
I think we should first add system tests for these types, before merging. As they are gonna be used to convert data from user types to the Spanner types, we better check how it's working in the case close to the real.
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.
@IlyaFaer Reverted to the previous convention, PTAL.
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.
I think this change is reasonable and looks a lot cleaner now. Just to have another pair of eyes, I'll add @larkee as a reviewer but if this is urgent, feel free to merge.
The proposed changes represent reworked
types.py
in accordance with the PEP-0249 Type Objects and Constructors specifications.Some of the classes implemented prior to the alpha release are not part of the DB-API standards. Those were left in for now but may be removed later in the GA release.