gabrieldemarmiesse / python-on-whales

An awesome Python wrapper for an awesome Docker CLI!
MIT License
537 stars 100 forks source link

Fix time argument formatting to include time zone #574

Closed kamalmarhubi closed 5 months ago

kamalmarhubi commented 5 months ago

In local testing, I saw that Container.logs misbehaved when passing the since parameter. I believe this is because my time zone is UTC-4, and so asking for something like

container.logs(since=container.state.started_at)

would return no logs as it was effectively asking for logs from the future:

I instrumented the Popen call to see what arguments were passed to the docker CLI. Taking the exact same command and appending a Z to the time to force the correct time zone printed logs as expected.

gabrieldemarmiesse commented 5 months ago

Many thanks for the thorough bug hunting! That's awesome! From what you described, it sounds like a good fix. I'll take a look in details and give you a review in the next few days :) I also wonder if this kind of bug can occur somewhere else in the codebase.

kamalmarhubi commented 4 months ago

@gabrieldemarmiesse yup I removed my monkey patch to test with current master and it works, thanks!