#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 )
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
andParser.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
andupload_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 10 months ago by
Description: | modified (diff) |
---|
comment:2 Changed 10 months ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
comment:3 Changed 8 months ago by
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 8 months ago by
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) ...
This is implemented as of [7e2299845b86].