crossbario / crossbar

Crossbar.io - WAMP application router
https://crossbar.io/
Other
2.05k stars 274 forks source link

fix localtime-to-utc mismatch to utc #1904

Closed antonymott closed 2 years ago

antonymott commented 2 years ago

cookies will flush correctly after setting "cookie": { "store": {..., "purge_on_startup": true }}

oberstet commented 2 years ago

I guess the value being compared against cookie['created'] is UTC as well?

also, not sure, some of the unit tests are failing in the CI .. might be a problem in the test ...

rgd the functions used/replaced

https://docs.python.org/3/library/datetime.html#datetime.datetime.now https://docs.python.org/3/library/datetime.html#datetime.datetime.utcnow

antonymott commented 2 years ago

Yes,

I think that's all it was, using .now() rather than .utcnow() for one side of the comparison caused the purge_on_startup to give unexpected results if user's crossbar router location was set to a timezone other than UTC, +/- the error just being difference between user's local computer and UTC.

As for production testing, I updated my own cookiestore.py with the fix and it's worked over multiple router restarts so I also wondered what was causing the unit tests to fail.

Best, Antony

On 11/16/21 03:53, Tobias Oberstein wrote:

I guess the value being compared against |cookie['created']| is UTC as well?

also, not sure, some of the unit tests are failing in the CI .. might be a problem in the test ...

rgd the functions used/replaced

https://docs.python.org/3/library/datetime.html#datetime.datetime.now https://docs.python.org/3/library/datetime.html#datetime.datetime.utcnow

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/crossbario/crossbar/pull/1904#issuecomment-970054592, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAYWV3FKDCIMV7F6JALYPFDUMILZLANCNFSM5ICR6KGA. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

om26er commented 2 years ago

CI failed and now this seems to be a real test failure, related to the changed code

antonymott commented 2 years ago

Looking over the failed test-amd64 (3.9) Details, does not reveal to me exactly where or why the code is failing. I can see the two unit test cookies each appear correctly formed with UTC “created” timestamps. Is there some more detailed error messaging that I can help look over to see where the problem is?

oberstet commented 2 years ago

Ok, reran the CI .. just in case. Same problem.

Not sure, haven't looked deeply, but the problem seem to be at least in this test crossbar.bridge.mqtt.test.test_tx.SendPublishTests.test_qos_2_resent_on_disconnect_pubrel:

https://github.com/crossbario/crossbar/blob/b9799d2ac6667a97373db0549d2055fff97d784c/crossbar/bridge/mqtt/test/test_tx.py#L1101

First differing element 0:
{'id': 'thisIsAnID', 'created': '2021-11-21T09:18:06.611Z[188 chars]: {}}
{'id': 'thisIsAnotherID', 'created': '2021-11-21T09:23:16[189 chars]: {}}

https://github.com/crossbario/crossbar/runs/4277393773?check_suite_focus=true#step:7:558

antonymott commented 2 years ago

So I tried (and subsequently closed) various small code tweaks to track CI errors, making each a small change and submitting pull requests to run the tests again. I left one PR open where I used the identical method to create utc timestamp both for create and flush. Tests failed again, but I thought you might want to look at the error on the most recent CI failure, as it was unexpected and seems unrelated to the code-fix: "ERROR: No matching distribution found for autobahn (unavailable)"

oberstet commented 2 years ago

rgd the CI, the one thing that is currently broken (for pypy) is docker image baking. the tests and everything else does work, pls have a look https://github.com/crossbario/autobahn-python/actions

rgd the other PR https://github.com/crossbario/crossbar/pull/1912 - closed that, because pls see comments there

anyways, sorry, I don't have time to dive into the issue, there is one I'm pretty sure, but I want to be sure we really fix it and don't introduce regressions ..