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

Another issue deserializing Interval #80

Closed
bzaks1424 opened this issue Sep 17, 2019 · 4 comments
Closed

Another issue deserializing Interval #80

bzaks1424 opened this issue Sep 17, 2019 · 4 comments
Assignees
Labels
bug Something isn't working

Comments

@bzaks1424
Copy link

Using 0.5.0 - I'm not sure how much work you want to go into this:

myproject - ERROR - Caught exception for <function getThing at 0x7f4098486d90>
Traceback (most recent call last):
  File "**Chalice-Project**/vendor/validator_collection/validators.py", line 812, in datetime
    value = datetime_.datetime.strptime(value, '%Y-%m-%d %H:%M:%S.%f%z')
  File "/usr/lib/python3.7/_strptime.py", line 577, in _strptime_datetime
    tt, fraction, gmtoff_fraction = _strptime(data_string, format)
  File "/usr/lib/python3.7/_strptime.py", line 359, in _strptime
    (data_string, format))
ValueError: time data '0:35:00' does not match format '%Y-%m-%d %H:%M:%S.%f%z'

Based on the chain of exceptions in the stack - it looks like you're rolling down a track of "potentially workable interval strings"

Have you thought about using something closer to this?
http://kbyanc.blogspot.com/2007/08/python-reconstructing-timedeltas-from.html

It's older code - but it looks like it would be able to help you parse through and instantly get your timedelta object.

In the mean time - I'm going to write my own function that's based on this and just do a on_deserialize for interval objects.

@bzaks1424
Copy link
Author

btw - this worked. I'm sure if you need help I could help figure out how to do this at a broader scale if I'm stuck at any point.

@insightindustry insightindustry added the bug Something isn't working label Sep 18, 2019
@insightindustry insightindustry self-assigned this Sep 18, 2019
@insightindustry
Copy link
Owner

insightindustry commented Oct 7, 2019

What's interesting about this bug is that I do not believe we should be attempting to parse Interval strings at all. When we deserialize datetime values, we do need to parse a variety of string formats to attempt to match, but that's not what should be happening when deserializing an Interval column type. Intervals should be serialized to the number of seconds (an integer value), and then deserialized to a timedelta based on the number of seconds, by default.

Can you provide some more background on this:

  1. What format are you serializing your objects to and then deserializing from?

  2. I'd like to double-confirm that you are in fact deserializing an Interval column type and not a datetime or time column type. Is that the case?

  3. Can you confirm the type of value (the input) that you are deserializing here when deserializing? Something as simple as the output of type(variable_name) would be helpful just to double-check the value's type.

@bzaks1424
Copy link
Author

In writing my API - my client side said they'd like to send (because of this):

PT2M15.445S

However - I've managed to get them to send some form of this:

"hh:mm:ss.SSSSSS"

And this is currently the code I'm running (Hopefully I don't have to worry about crossing days... :-D):

from dateutil import parser
import datetime
def parse_timedelta(Interval):
    today = datetime.datetime.combine(
        datetime.date.today(), datetime.datetime.min.time())
    intervalTime = None
    try:
        intervalTime = parser.parse(Interval)
    except Exception:
        return None
    return (intervalTime - today)

@insightindustry
Copy link
Owner

Hrm. So it turns out this issue is a little trickier than initially diagnosed.

On the one hand, as mentioned above, one (part) of the issue is that you were trying to coerce a string that expressed an amount of time (e.g. 00:35:00) to a Python timedelta object which is the downstream Python representation interpretation of SQLAlchemy's Interval data type. This would always fail, because SQLAthanor did not know how to coerce that kind of a string into a timedelta. That issue was fixed by creating a validator function in the Validator-Collection library that can coerce that kind of string to a corresponding timedelta object.

However, there's a second layer of problem that is occurring here, which is that whether an Interval column is represented as a datetime.timedelta object or as a straight datetime.datetime depends on the SQL Engine being used. When not using PostgreSQL, the Interval SQL type seems to be converted to DATETIME. However, the (implicit) behavior of a native DATETIME attribute versus a "converted" DATETIME attribute is different: a native DATETIME should accept a datetime.datetime object, while a "converted" DATETIME attribute needs to first perform some date arithmetic relative to the epoch date to properly apply the amount of time represented by an interval.

The date arithmetic and the type conversions are the easy part - the hard part is figuring out how to differentiate a native DATETIME attribute from a "converted" DATETIME attribute by introspecting the relevant SQLAlchemy internals. Still working to figure this out, I'm afraid.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants