-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
fix for unpickleable datetime tzs set by snowflake #1670
fix for unpickleable datetime tzs set by snowflake #1670
Conversation
fixed_row = [] | ||
for col in row: | ||
if isinstance(col, datetime.datetime): | ||
col = col.astimezone(tz=pytz.UTC) |
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 like this approach a lot: force-converting everything to UTC seems fine to me and this is the right place to do it I think (I'd rather this than have RPC be special and funky, if we can avoid it, right?)
The only suggestion I have is that maybe the base class should do the [dict(zip(column_names, row)) for row in results]
in its process_results
, and snowflake should just do this and then return super(SnowflakeConnector, self).process_results(fixed)
at the end? That's more of a style thing though and I don't have strong feelings about it, converting to a dict just feels like it's definitely "processing results".
909472f
to
fe3986b
Compare
hey @cmcarthur - mind giving this one a look when you get a chance? I ended up porting the code from snowflakedb/snowflake-connector-python#188, as this will likely be a quicker route to getting this code into production. |
return datetime.timedelta(0) | ||
|
||
def __repr__(self): | ||
return self.name |
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.
don't define this class -- use pytz.FixedOffset
instead
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.
:)
code looks fine, but flake8 is broken |
e78a08d
to
50fa1ba
Compare
The
snowflake-connector-python
library does something... surprising... when handling timezones. It dynamically generates its own timezone objects which are not pickleable. As a result, simple queries like:will fail if run on the rpc server. This happens specifically when data is passed between subprocesses over a queue.
Relevant issue: snowflakedb/snowflake-connector-python#81
Relevant code: converter.py#L140
I'm unsure if the approach I took here is a good one. I think I'd prefer for this code to live closer to the RPC server, but wasn't exactly sure if we had a good place to put this today. Very open to feedback / insight here.