Migrate from pytz to zoneinfo, list (close to) all time zones

[hroncok]

https://listman.redhat.com/archives/anaconda-devel-list/2021-February/msg00020.html

This is very much untested. I've written it and never actually attempted to run it. Not yet anyway.

I found a problem thou. Python's zoneinfo does not have any concept of common timezones. I can implement my own method to determine whether a timezone is common (pytz uses zone.tab and a short list of hardcoded values).

With pytz 2021.1, the extra zones effectively added by this PR are:

>>> import pytz, zoneinfo
>>> pytz_zones = {tz for tz in pytz.common_timezones if '/' in tz and not tz.startswith('Etc/')}
>>> zoneinfo_zones = {tz for tz in zoneinfo.available_timezones() if '/' in tz and not tz.startswith('Etc/')}
>>> len(zoneinfo_zones) / len(pytz_zones)
1.1762013729977117
>>> zoneinfo_zones - pytz_zones
{'Africa/Asmera',
 'Africa/Timbuktu',
 'America/Argentina/ComodRivadavia',
 'America/Atka',
 'America/Buenos_Aires',
 'America/Catamarca',
 'America/Coral_Harbour',
 'America/Cordoba',
 'America/Ensenada',
 'America/Fort_Wayne',
 'America/Godthab',
 'America/Indianapolis',
 'America/Jujuy',
 'America/Knox_IN',
 'America/Louisville',
 'America/Mendoza',
 'America/Montreal',
 'America/Porto_Acre',
 'America/Rosario',
 'America/Santa_Isabel',
 'America/Shiprock',
 'America/Virgin',
 'Antarctica/South_Pole',
 'Asia/Ashkhabad',
 'Asia/Calcutta',
 'Asia/Chongqing',
 'Asia/Chungking',
 'Asia/Dacca',
 'Asia/Harbin',
 'Asia/Istanbul',
 'Asia/Kashgar',
 'Asia/Katmandu',
 'Asia/Macao',
 'Asia/Rangoon',
 'Asia/Saigon',
 'Asia/Tel_Aviv',
 'Asia/Thimbu',
 'Asia/Ujung_Pandang',
 'Asia/Ulan_Bator',
 'Atlantic/Faeroe',
 'Atlantic/Jan_Mayen',
 'Australia/ACT',
 'Australia/Canberra',
 'Australia/Currie',
 'Australia/LHI',
 'Australia/NSW',
 'Australia/North',
 'Australia/Queensland',
 'Australia/South',
 'Australia/Tasmania',
 'Australia/Victoria',
 'Australia/West',
 'Australia/Yancowinna',
 'Brazil/Acre',
 'Brazil/DeNoronha',
 'Brazil/East',
 'Brazil/West',
 'Canada/Saskatchewan',
 'Canada/Yukon',
 'Chile/Continental',
 'Chile/EasterIsland',
 'Europe/Belfast',
 'Europe/Nicosia',
 'Europe/Tiraspol',
 'Mexico/BajaNorte',
 'Mexico/BajaSur',
 'Mexico/General',
 'Pacific/Johnston',
 'Pacific/Ponape',
 'Pacific/Samoa',
 'Pacific/Truk',
 'Pacific/Yap',
 'US/Aleutian',
 'US/East-Indiana',
 'US/Indiana-Starke',
 'US/Michigan',
 'US/Samoa'}

That's 17.6% of extra zones.

Please let me know if this actually matters or not.

[VladimirSlavik]

I found a problem thou. Python's zoneinfo does not have any concept of common timezones.

I have run into at least one RFE that depends on not simplifying the zone list to the common ones. (The Montreal thing by @lsatenstein.) Unless we find some technical or usability reason to simplify the list, I would be most happy to see the full list.

The main doubt I had was the ability to map the zones back onto the map-of-the-world thingy.

[VladimirSlavik]

Please rebase and re-target to f34-devel? There's nothing on master so it should be no work at all.

[VladimirSlavik]

Tests are mostly broken right now, so no point enabling them :(

[hroncok]

You want this in f34 already? Fine by me.

[hroncok]

Tests are mostly broken right now.

How do I check easiest if this even runs? I see you use packit. Does it produce rpms I can install in a Fedora 34 VM?

[poncovka]

/test

[hroncok]

Before this PR:

After this PR:

The UI seems to do weird things, e.g. if I try to select Australia/Canberra, it selects Australia/Sydney etc. Or if I select United States, it gives me Americas instead and I need to to select United States 3 to 4 times in order to actually see the options there. Not sure if it is related to my PR or not.

[VladimirSlavik]

I think that's the map widget, just as I feared - it doesn't know where some of the time zones actually lie. Alternatively, it is concerned only with time zones, not locales, and swaps them for the most current definition. Or it is Anaconda's doing. I'm literally writing here ideas as I browse the data, so feel free to draw your own conclusions, that might be better. https://github.com/dashea/timezonemap

PS: Did you swap the labels for before and after?

PPS: The first Americas thing does not get a pin either way. I think the data there are just out of date. Maybe the way to go forward is to identify the gaps, and patch that thing first. Or filter what we show (shudder).

[VladimirSlavik]

Sorry for recurrent brain dumps, but here I go once more. I think there's some way to tell if a TZ is an alias or not - that might be the key to getting rid of the auto-switching ones. IIRC I saw code for such filtering in Anaconda, so this might be a similar but subtly different problem.

[VladimirSlavik]

Oh look, here we go: https://github.com/dashea/timezonemap/blob/master/src/data/backward Explains the Canberra -> Sydney at least a bit.

[VladimirSlavik]

It might be better to move the sorting into function itself to take advantage of the cache.

[hroncok]

Technically, it could. However the other function that uses this does not need sorting and it appeared to me that get_all_regions_and_timezones() is usually called just once? If it is called multiple times, should that be cached as well?

[M4rtinK]

@hroncok Thanks for the PR! Will try to respond to individual points bellow. :)

• extra timezones

That's a good point, I wonder if just having the extra timezones would be a problem - the list in the GUI is already long, but 17% extra zones would hardly make it worse. Also as @VladimirSlavik mentioned, there have been RFEs about missing zones.

If we really need to have some custom list of zones to include or exclude for some reason, I think it should really not be hardcoded in Anaconda source code but instead live in langtable (https://github.com/mike-fabian/langtable) like all similar language/locale/keyboard mapping things we use. Still in such a case I would arrange for that, can't really burden you with that extra work. :)

• testing the changes

Seeing the screenshots I see you got it working, but for the record - for interactive debugging like this editing the Anaconda files on the live image is likely best. It should be possible to start sshd on a booted live image and overwrite the Anaconda files as needed. The just relaunch Anaconda using the launcher icon after every edit.

Alternatively it's possible to use the updates image mechanism. That's a bit more involved but can be automated, starts with clean environment every time and can be used to test the netinst image. I can get you started on this if you want. :)

• map widget issues

Let's say that this has been fragile and full of edge cases already. :P Still I guess the only thing really changing is that some timezones might be called differently & there could be more of them. And if there are already some mappings in the timezonemap projects maybe these could be adjusted to match the new names ?

[hroncok]

What do I do here?

[VladimirSlavik]

@hroncok I'm not sure, but we'll figure it out. Unfortunately I'm not able to start digging into this right away.

Assuming the widget really needs some dusting off and updating: Somebody would have to figure out what goes bonkers in that thing due to missing data, and get a solid idea what needs to be fixed there. Then we can start with the PRs... I am aware that that's far more than just rewriting bits of anaconda, so maybe that would be a task for me?

[VladimirSlavik]

After taking a slightly longer look at this, I think it's fine. @hroncok please rebase to master && change target branch, and I'll take care of tests.

If a "link" timezone is selected, GUI changes it to the "target" one. That seems the only potential point of contention...

[VladimirSlavik]

After discussing with Miro, I went ahead with the changes required to get this done. Rebased this to master, force-pushed, and retargeted. May need another force-push to re-run tests and clean up GH statuses, it's a total mess :(

[VladimirSlavik]

/kickstart-test --testtype smoke

[VladimirSlavik]

Not sure what the codecov problem is, it links to some cute but generic error message.

[VladimirSlavik]

/kickstart-test --testtype time

[VladimirSlavik]

Forgot this bit.

[jkonecny12]

LGTM!

[hroncok]

Awesome!

[VladimirSlavik]

Haha, you are too.

[AdamWill]

I think this has caused a problem with daylight savings. openQA is showing the clock exactly an hour fast on fresh installs; our official openQA deployments are in the Eastern timezone, which uses daylight savings. This is, I think, causing two tests to fail (in non-obvious ways it took me a while to trace back to the wrong time and then to this commit). These problems started in exactly the compose this commit ultimately landed in - Fedora-Rawhide-20210520.n.1.

I'm looking through the changes now to see if I can spot the issue - I think I actually touched the set_system_date_time code last before this PR - but it's subtle stuff and I forget the details...

[AdamWill]

OK, yeah, it's definitely this commit. Here's a test script I ginned up to compare set_system_date_time from before this PR and set_system_date_time from after it, using three timezones: UTC, US/Eastern, and Asia/Kolkata. Both versions get the right timestamp for UTC and Asia/Kolkata, because neither of those timezones has daylight savings...but the new version gets the wrong timestamp for US/Eastern, which does do daylight savings.

import datetime
import pytz
import time
import zoneinfo

def get_timezone_new(timezone):
    """
    Return a tzinfo object for a given timezone name (new way).
    """
    return zoneinfo.ZoneInfo(timezone)

def get_timezone_old(timezone):
    """
    Return a tzinfo object for a given timezone name (old way).
    """
    return pytz.timezone(timezone)


def set_system_date_time_new(tz=None):
    """
    Set system date and time to "now" (new way).
    """
    utc = zoneinfo.ZoneInfo(key='UTC')
    # If no timezone is set, use UTC
    if not tz:
        tz = utc
    else:
        tz = get_timezone_new(tz)

    time.tzset()

    # get the right values
    now = datetime.datetime.now(tz)
    set_date = datetime.datetime(now.year, now.month, now.day, now.hour, now.minute, now.second, tzinfo=tz)

    # Calculate the number of seconds between this time and timestamp 0
    epoch = datetime.datetime.fromtimestamp(0, tz=utc).astimezone(tz)
    timestamp = (set_date - epoch).total_seconds()
    print(f"New timestamp: {timestamp}")

def set_system_date_time_old(tz=None):
    """
    Set system date and time to "now" (old way).
    """

    utc = pytz.UTC
    # If no timezone is set, use UTC
    if not tz:
        tz = utc
    else:
        tz = get_timezone_old(tz)

    time.tzset()

    # get the right values
    now = datetime.datetime.now(tz)
    set_date = tz.localize(datetime.datetime(now.year, now.month, now.day, now.hour, now.minute, now.second))

    # Calculate the number of seconds between this time and timestamp 0
    epoch = utc.localize(datetime.datetime.utcfromtimestamp(0)).astimezone(tz)
    timestamp = (set_date - epoch).total_seconds()
    print(f"Old timestamp: {timestamp}")

print("Testing UTC:")
set_system_date_time_new()
set_system_date_time_old()
print("Testing US/Eastern:")
set_system_date_time_new("US/Eastern")
set_system_date_time_old("US/Eastern")
print("Testing Asia/Kolkata:")
set_system_date_time_new("Asia/Kolkata")
set_system_date_time_old("Asia/Kolkata")

[hroncok]

Thanks for the analysis and proposed fix @AdamWill!