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

TimeZone Validation Error when bulk-editing sites #2369

Closed
bdlamprecht opened this issue Aug 17, 2018 · 7 comments
Closed

TimeZone Validation Error when bulk-editing sites #2369

bdlamprecht opened this issue Aug 17, 2018 · 7 comments
Labels
status: accepted This issue has been accepted for implementation type: bug A confirmed report of unexpected behavior in the application

Comments

@bdlamprecht
Copy link
Contributor

Environment

  • Python version: 3.6
  • NetBox version: 2.4.3

Steps to Reproduce

I'm not sure exactly how this happened, but the time_zone of a certain number of sites are really messed up in my production instance of NetBox.


It appears to only be an issue where, the offset is NOT on the even hour mark (i.e. UTC +05:30 in India).
When I try to bulk-edit an unrelated custom_field, I get the following error:

timezone error


However, when I view that site, everything appears to be normal:

timezone error 2


Only after clicking Edit and not changing anything and then clicking Save and re-trying the bulk-edit I was attempting before, does the problem go away.

Expected Behavior

To NOT see a TimeZone validation error when the WebUI appears to be okay.

Observed Behavior

The validation error seems to include an additional 53 minutes to the timezone no matter what until after the site is individually edited and saved.

I'm going to proceed and keep fixing my production instance of NetBox so I mostly likely won't have an option to follow-up on this should you require more information. I'll try and get a backup of my postgres database to which contains these errors, but I don't know if that will help at all.


Here's another example of a different site in India:

timezone error 3

Showing the individual site:

timezone error 4

@bdlamprecht
Copy link
Contributor Author

I just found another site where the offset was NOT on the 30 minute mark UTC +0700:

timezone error 5

And the individual site:

timezone error 6

The timezone in the error is off by 7 minutes instead of 53 like before, so there is definately some strange math going on internally that is wrong.

@bdlamprecht
Copy link
Contributor Author

Here another odd error (-1 day?):

timezone error 7

timezone error 8

@jeremystretch
Copy link
Member

So after a bit of poking, it seems that the root issue is that django-timezone-field restricts the choices for this field to the timezones included in the set pytz.common_timezones. The API field serializer, however, will accept any string which pytz considers a valid timezone (i.e. pytz.all_timezones).

I don't want to inherit the problem of maintaining timezones. Really, I can't overstate how much I'd like to avoid that. So we have two options:

  1. Tweak the API serializer to only accept values within pytz.common_timezones.
  2. Initialize TimeZoneField so that all of pytz.all_timezones are valid choices.

I don't have a strong opinion, although it's worth noting that all_timezones is a considerably larger set:

>>> len(pytz.common_timezones)
439
>>> len(pytz.all_timezones)
591

@jeremystretch jeremystretch added type: bug A confirmed report of unexpected behavior in the application status: under review Further discussion is needed to determine this issue's scope and/or implementation labels Aug 17, 2018
@bdlamprecht
Copy link
Contributor Author

I agree with this comment:

I don't want to inherit the problem of maintaining time-zones. Really, I can't overstate how much I'd like to avoid that.

Concerning the options you outlined above, I'd personally prefer doing option #2 as py.all_timezones appears to include "odd" ones that I'm currently using.

However, it you do choose option #1, I'll deal with it as I don't want to place my problems of getting accurate data from my customer (:grimacing:) on the NetBox project and make it less desirable for the community as a whole.

All that being said, I don't know how much of a burden of an additional 152 time zones is (~34% of the current total)?

@jeremystretch
Copy link
Member

Here's the delta of additional timezones. Some of them seem a bit confusing. For example, GMT+0, GMT-0, and GMT0 are all choices. Also there are items like Singapore, which is presumably a duplicate (but maybe not) of Asia/Singapore.

@bdlamprecht
Copy link
Contributor Author

I think it would be beneficial to include these additional entries to as to not be too restrictive on end-users.

@jeremystretch jeremystretch added status: accepted This issue has been accepted for implementation and removed status: under review Further discussion is needed to determine this issue's scope and/or implementation labels Aug 20, 2018
@jeremystretch
Copy link
Member

I'm just going to limit the serializer to accept only values from within pytz.common_timezones for now. I'd also like to organize the timezones by country to make the list a bit more user-friendly. Maybe we can revisit expanding the list once that's in place.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: accepted This issue has been accepted for implementation type: bug A confirmed report of unexpected behavior in the application
Projects
None yet
Development

No branches or pull requests

2 participants