ValueError on most offer requests api calls
Closed this issue · 11 comments
Hi there :)
I'm getting a ValueError("time data '2023-05-13T03:59:00.000Z' does not match format '%Y-%m-%dT%H:%M:%SZ'") on most of my api calls with the python client.
Here is the code to reproduce:
from duffel_api import Duffel
duffel = Duffel(access_token="MY_TOKEN")
response = (duffel.offer_requests.create()
.cabin_class("economy")
.passengers([{"type": "adult"}])
.max_connections(2)
.return_offers()
.slices([
{"origin": "PAR", "destination": "LON", "departure_date": "2023-06-14"},
{"origin": "LON", "destination": "PAR", "departure_date": "2023-06-16"},
]
)
.execute())Expected behavior
No exception is raised
System
- OS: MacOS
- Library version: 0.5.0
- Python Version: 3.11.2
Same issue here on Windows and all versions of Python 3.8-11.
The issue seems to be inconsistent date formatting in the API; dates sometimes have milliseconds and sometimes don't.
lines 104
lambda value: datetime.strptime(value, "%Y-%m-%dT%H:%M:%SZ")
and 245-55 in offer.py
arriving_at=datetime.strptime(json["arriving_at"], "%Y-%m-%dT%H:%M:%S"), departing_at=datetime.strptime(json["departing_at"], "%Y-%m-%dT%H:%M:%S"),
don't seem to account for this. There's a function in utils parse_datetime that handles this situation (just "parse_datetime" seems to work as a replacement for 104, and was used in line 99 preceding it, so I'm unaware of why 104 was left with strptime), but regarding 245-55, it expects a "Z", assuming UTC.
If it's to be assumed that arriving_at and departing_at are in UTC, then maybe you could just append a "Z" in parse_datetime if it doesn't yet exist (this, alongside the earlier changes removed errors for me, at least at a glance). But it won't pass test_parse_datetime which expects a value error to be raised if there's no "Z". Not sure if this matters if everything using it actually is UTC, but I can't tell if that's the case. This is just my best understanding of what's going on.
Hi folks, we'll take a look at this shortly. We do have some differing datetimes that we try to resolve through auto resolution in the libraries and we clearly missed something here.
Thanks for the report.
@felixmeziere oddly enough I don't seem to be able to trigger this. If you can still trigger it, would you be so kind as to intercept the data payload and send it over?
I'll keep looking at how this inconsistency is coming through for you but not for me.
@brpeterson the second example you gave:
arriving_at=datetime.strptime(json["arriving_at"], "%Y-%m-%dT%H:%M:%S"),
departing_at=datetime.strptime(json["departing_at"], "%Y-%m-%dT%H:%M:%S"),
Although they might seem inconsistent, in this case that format should be correct. For arrival and departure, we don't have timezones because it's always the local time for where you're either departing or arriving. (This isn't to say we may be missing a trick somewhere but that's why they come without timezone information)
Hi @nlopes thanks for looking into this! I'm still getting this all the time in Sentry so I guess it'll be easy to reproduce. Here is the stack trace in Sentry with the data that triggered it:
@nlopes thanks for the quick response! I just did a call that reproduces this locally:
duffel = Duffel(access_token=settings.DUFFEL_API_KEY)
(
duffel.offer_requests.create()
.passengers([{"type": "adult"}])
.max_connections(2)
.return_offers()
.slices(
[{'origin': 'PAR', 'destination': 'BER', 'departure_date': '2023-06-06'}, {'origin': 'BER', 'destination': 'PAR', 'departure_date': '2023-06-08'}]
)
.execute()
)Is this enough, or do you need more?
I still can't reproduce (which is not great) but I've pushed #275. If you could try with the branch in that PR and let me know the results I'd be forever grateful!
Damn, ok will check right now, thanks
@nlopes just tried with your PR's branch, so far it seems to be working with it, thanks!
I'm happy to jump on a call if needed.
This auto closed but we'll try to get a new version out soon.