Opened 10 months ago

Closed 7 months ago

Last modified 5 months ago

#134 closed defect (fixed)

Assume UTC for directory listings from server

Reported by: schwa Owned by: schwa
Priority: major Milestone: 4.0.0
Component: Library Version: 3.4
Keywords: server listing utc timezone time shift Cc:

Description (last modified by schwa)

For a long time, ftputil has assumed that FTP servers present dates and times in directory listings in "their" timezone. I think this is a problem for several reasons:

  • ftputil can't know which timezone the server is in. Even if the user specifies a time shift, it's a constant value and may not apply to dates/times in the past or in the .
  • Having a server use UTC for its timestamps makes sense because using a timezone may lead to skipped or double hours for daylight saving time switches. So I think the assumption is justified anyway that most servers will use UTC.

Therefore, ftputil should assume the server uses UTC in listings. Of course, this assumption may be wrong, but in that case there's still the time shift concept (local server time minus local client time). If the server time in listings is actually UTC, the time shift is zero (see also next paragraph). If the server doesn't use UTC, a time shift can still be set with FTPHost.set_time_shift.

On a related note, os.path.getmtime returns the time as seconds since the epoch (i. e. UTC), so comparing (UTC) timestamps from the server and the client become trivial, apart from the precision of the server times.

Subtasks:

  • Change code for Parser.parse_unix_time and Parser.parse_ms_time to assume UTC. Adapt tests.
  • Change code in file_transfer.source_is_newer_than_target and adapt tests.
  • Possibly adapt tests for download_if_newer and upload_if_newer.
  • Update documentation. Include a more thorough explanation of the problems with local times and why UTC is the best bet.

This change is backward-incompatible. Because I came to the above conclusion only now, there hasn't been a deprecation warning about this change in past ftputil versions. However, I think it would be overkill to release a version ftputil 3.5 only to add a deprecation warning before the changes. Also, normally the impact of this change should be small since most people will use download_if_newer and upload_if_newer, which hide the exact definitions of the individual timestamps of server and client.

Change History (4)

comment:1 Changed 7 months ago by schwa

Description: modified (diff)

comment:2 Changed 7 months ago by schwa

Resolution: fixed
Status: assignedclosed

This is implemented as of [7e2299845b86].

comment:3 Changed 5 months ago by schwa

Can we offer users a relatively simple migration path?

Can we give users a recipe to change their old code to be compatible with the new above behavior?

The previous approach was assuming that the server would return listings in the timezone of the client.

Ideas:

  • Give the user a function that determines a time shift value that must be passed to set_time_shift to get the old 'default' behavior. This function shouldn't be in the ftputil code, only in the documentation.
  • Maybe building on the previous recipe, tell the user how to create a derived FTPHost class that would preset the time shift so that ftputil would behave similar as before.

Caveat: However the method of setting the time shift, this is only for the default time shift. The semantics of the time shift value is not changed to that of the old ftputil version. Specifically, setting a particular constant value with set_time_shift with the same server behavior will lead to different "perceived" server times for the old and new ftputil version.

Look more into this.

comment:4 Changed 5 months ago by schwa

Description: modified (diff)

Reminder: If the user has write permissions for the current directory on the FTP server, they can call ftp_host.synchronize_times() to set the time shift automatically.

If that's not possible or desired and the server reports timestamps in the same timezone as for the client, the time shift can be set with

ftp_host.set_time_shift(
  round( (datetime.datetime.now() - datetime.datetime.utcnow()).seconds, -2 )
)

I thought about different approaches to make it even easier for users, for example, supporting a class attribute like initial_time_shift ("utc", "same_timezone_as_client", float) or a similar argument for the constructor. However, all these approaches add very little to the obvious implementations a user would/could do.

If the default time shift should be set on the class level, use

class MyFTPHost(ftputil.FTPHost):

    def __init__(self, *args, **kwargs):
        super().__init__(*args, **kwargs)
        my_initial_time_shift = ...
        self.set_time_shift(my_initial_time_shift)

If the default time shift should be set on the instance instance level, use

with FTPHost(...) as host:
    my_initial_time_shift = ...
    host.set_time_shift(my_initial_time_shift)
    ...
Note: See TracTickets for help on using tickets.