Hi ~lorenzom,

Many thanks for your feedback! :-)

If the date looks like it is in the future (either due to time-sync-issues or whatever reason), we have to assume that the date belongs to the previous year. Fine. Since this logic is supposed to work for any date, it should also work for Feb 29. As seen, it is not possible to just subtract one year on Feb 29 🤣

When the date looks like it is from the future, and this date is Feb 29, should not subtract 1 from current year but find and use the previous leap year instead. This formula always finds the previous leap year for a given year: (year - 1) // 4 * 4.

I think that doesn't really help.

In my opinion, the goal shouldn't be to use the "same" date (same day and month), but to find a "correct" date (or "as correct as possible"). Subtracting four years would obviously give you a date four years in the past whereas in all other cases we go back one year. A slightly better choice, I think, would be to choose a date close to "February 29th, but one year ago". For example, you could pick March 1st of the previous year. This is similar to what some people do whose birthday is on February 29th: in non-leap years, they celebrate their birthday on March 1st.

That said, I think neither of these approaches is ok. Yes, they'd avoid the exception, but we'd use a date that we know is incorrect. On the other hand, the current ftputil code works if the time shift is set correctly.

Now, I suspect that relatively few people read the documentation attentively enough and aren't even aware that they're supposed to set the time shift. That's not really a complaint. Everyone, including me, cuts corners in some situations. Nobody's perfect.

Let's look at the time shift-related use cases, which I think are relevant for our discussion:

• If you want to use methods like upload_if_newer and download_if_newer, you must set a correct time shift. Otherwise these methods won't work correctly. The only exception is if your current time zone is +00:00, in which case the default time shift of 0.0 happens to be correct.
• You only want to get the directory or file names in a directory, or more generally, you're not interested in the last modification datetime at all. You also don't want to use upload_if_newer or download_if_newer (see the previous case). You also never rely on the datetime returned by FTPHost.stat or similar methods.

For the second use case, we only want to avoid that we get an exception when parsing a directory. In this case, either your approach or mine (picking March 1st) would work.

However, this ignores the first use case. Here, we'd avoid the exception, but we'd use a timestamp we know to be wrong. I can't help it, but in my opinion, that's worse than assuming the time shift is set correctly and use wrong timestamps because of that (although in both cases we use a wrong timestamp, no more, no less).

After thinking about the problem for a while (which is the reason I didn't reply earlier), I think the problem is that since the time shift is set to 0.0 by default, ftputil can't distinguish between these cases:

• The time shift actually is 0.0, left at the default.
• The time shift actually is 0.0, and was explicitly set to this value.
• The time shift was never set, but actually is not 0.0.

At the moment, I think the cleanest way out of this would be:

• When creating an FTPHost object, set the time shift to a special value like UNSET_TIME_SHIFT.
• If this default is present, parsing a directory line will set the timestamp to None since no timestamp can be determined. This would support the use case where you're not interested in the timestamps (neither implicitly nor explicitly).
• If this default is present and you use upload_if_newer or download_if_newer, ftputil will raise an exception, complaining about the unknown time shift.
• If the time shift is determined with synchronize_times or set explicitly with set_time_shift, ftputil will behave as it does currently: You get timestamps corrected by the time shift and parsing directory entries always works (as long as the time shift is set correctly and the server doesn't actually use timestamps in the future ;-) ).

Using this approach, it would mean that accidental uses of the default time shift 0.0 result in an exception instead of bogus results and behavior. However, in cases where up to now the time shift was intentionally left at or was set to 0.0, this is a breaking change.

Unless we come up with a better idea, I suggest the following:

• I'll (hopefully soon, but it may take a while) release ftputil 5.2, which, if the time shift is used and is 0.0, will print a deprecation warning, but won't otherwise change the behavior.
• Later, I'll release ftputil 6.0 (new major version number because I follow SemVer and this is a breaking change) with the UNSET_TIME_SHIFT approach described above.

As I said, this would be a breaking change for users who deliberately used a time shift of 0.0. (Although I think, using set_time_shift explicitly would be cleaner.)

(There are also edge cases where you actually can ignore wrong timestamps. If you have a job on the FTP server that adds new files every week and you have a script that runs (for example) three days after the files were added, time zone differences don't matter. For example, download_if_newer will detect that the remote and local timestamps for the new files are more than a day apart (regardless of the time shift). I only mention this edge case here for completeness. I don't think it should change anything in the chosen release strategy.)

I wanted to submit a pull request, but I have no clue how to do it on sourcehat.

The approach recommended by Sourcehut is described here, but personally I don't mind getting a text diff, assuming the changes aren't too large. And if they're large, it would anyway make a lot of sense to contact me first before working on a PR. :-)

Regarding PRs on Github, I'd deactivate them if I could, but it seems that's not possible. Unfortunately, you can deactivate issues and wikis, but not PRs.

I found a mirror repo on GitHub, but this seems to be very old code.

Sorry about that. The Github repo is supposed to be an up-to-date mirror. I thought I had my Git remotes set up so that git push pushes both to Sourcehut and Github, but I confused this with another repo where I had this set up like that. I fixed the remotes now and pushed the current state of my repository so Sourcehut and Github are in sync.

Hello Stefan,

our Python projects which uses ftputil were also affected on Feb 29. It was kind of stressful day. We could mitigate the issue by calling FTPHost.synchronize_time(). I didn't even know this was a thing 😅.

Assuming that I correctly unterstand the logic behind subtracting 1 from current year when the date looks like it's in the future (stat.py:299), I think there is still a bug in the code:

If the date looks like it is in the future (either due to time-sync-issues or whatever reason), we have to assume that the date belongs to the previous year. Fine. Since this logic is supposed to work for any date, it should also work for Feb 29. As seen, it is not possible to just subtract one year on Feb 29 🤣

When the date looks like it is from the future, and this date is Feb 29, should not subtract 1 from current year but find and use the previous leap year instead. This formula always finds the previous leap year for a given year: (year - 1) // 4 * 4. You can run the code here to verify that the formula works correctly: https://gist.github.com/lollo0296/be6b950aabdc899827647d7987304c7d

I wanted to submit a pull request, but I have no clue how to do it on sourcehat. I found a mirror repo on GitHub, but this seems to be very old code. For this reason, I will submit my proposal here:

#Current code (stat.py:299):

if self._datetime(
                server_year, month, day, hour, minute, 0
            ) > server_now.replace(second=0) + datetime.timedelta(seconds=120):
                server_year -= 1

#My change proposal:

if self._datetime(
                server_year, month, day, hour, minute, 0
            ) > server_now.replace(second=0) + datetime.timedelta(seconds=120):
                if month == 2 and day == 29:
                    server_year = (server_year - 1) // 4 * 4 
                else:
                    server_year -= 1

Cheers 🍻