search

kytos-ng / maintenance

Kytos Maintenance Window NApp
https://kytos-ng.github.io/api/maintenance.html
0 stars 7 forks source link

Discussion about adding support to different timezones #66

Open italovalcy opened 1 year ago

italovalcy commented 1 year ago

Hi,

Trying to capture the discussion on PR #64:

@viniarck viniarck 3 weeks ago

@Ktmi, for these cases flask provides jsonify (that we also use in other NApps in the responses instead of building and hooking into reponse_class), I think the intention would be clearer and cleaner with it:

    maintenances = self.scheduler.list_maintenances().dict()["__root__"]
    return jsonify(maintenances), 200

Could you adapt this? Same comment for /v1/ below.

@Ktmi Ktmi 3 weeks ago

I used pydantics jsonfiy specifically to match TIME_FMT we are using. If we remove the requirement to match TIME_FMT, then we can perform marshalling with flask. This should also fix the issue with submitting times that don't include tz info.

@viniarck viniarck 3 weeks ago

I see. So, this won't be a blocker for this PR, since the TIME_FMT with %z was already in place historically, but since we're stabilizing the v1 of this NApp, I'll encourage you to continue this discussion, supporting timezones other than UTC with the %z offset fmt can really complicate things and make it harder to maintain and also to correlate events (good thing that on logs we'll have UTC on each log entry). Maybe we can nuke pytz and only support UTC in the v1 of this NApp? It would be great to confirm and have a discussion with network operators. In the old blueprint draft (that will be refined) on issue https://github.com/kytos-ng/maintenance/issues/59 we also didn't have any requirement to support arbitrary timezones.

We've been using UTC everywhere on NApps and core (core also natively supports encoding datetime with this "%Y-%m-%dT%H:%M:%S format.