Preventing the Python 3.12's "Deprecated since version 3.12: Use datetime.now() with UTC instead."

[Lisias]

Description

See #913

Types of changes

• Replay fix (fixes a replay specific issue)
• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added or updated tests to cover my changes.
• All new and existing tests passed.

[Lisias]

I made a mistake, forgot that WARCIO is also using naive datetimes. I'm working on a short term solution, please wait.

[Lisias]

I finally got my stuff together and remembered this one.

I just realized that WARCIO was updated:

webrecorder/warcio#185

(yeah, I'm a bit slow... 😬 )

I'm revising this code and comparing it with whatever I have something in (my custom) production at this moment - that code is working for months already, time more than enough to had bitten my back if I had made some serious mistake.

[Lisias]

That's the history: With WARCIO 1.7.5 giving us the option for getting non naive datetimes from it, after this pull request is applied we would have internally code using both datetime types: the deprecating naive ones, and the new canon tz aware ones.

This would be a problem on code touched by the previous commit, and so code reviewing the changes I found that pywb.warcserver.access_checker and pywb.warcserver.resource.responseloader would end up handling both datetimes concurrently - what could lead to undesired surprises on the long run.

So I changed these, adding True to the calls to WARCIO's timestamp_to_datetime at:

• pywb/pywb/rewrite/templateview.py

  Line 247 in 7b0f8b5

  value = timestamp_to_datetime(value)

• pywb/pywb/warcserver/access_checker.py

  Line 118 in 7b0f8b5

  embargo['before'] = timestamp_to_datetime(str(value))

• pywb/pywb/warcserver/access_checker.py

  Line 122 in 7b0f8b5

  embargo['after'] = timestamp_to_datetime(str(value))

• pywb/pywb/warcserver/resource/responseloader.py

  Line 305 in 7b0f8b5

  dt = timestamp_to_datetime(cdx['timestamp'])

• pywb/pywb/warcserver/resource/responseloader.py

  Line 585 in 7b0f8b5

  dt = timestamp_to_datetime(cdx['timestamp'])

Doing a second code review, this time worrying about the consequences of the changes, I realized that these changes are not needed because the datetime generated are fed back into WARCIO and then forgot about - and so there's no possible clash between naive and tz aware datetimes - and that last commit of mine is useless.

However, I had forgot that updating my branch would reflect automatically on this pull request, my bad. So I decided to rebase the branch back one commit, and ask first if you guys would prefer to have the code above changed due some internal compliance, or if whatever we have now is good to go.

I'm marking this pull request as Ready for Review anyway - as I found no possible problem with WARCIO with or without getting naive datetimes from them.

--- edit: some typos fixed.

[tw4l]

Thank you for this PR, and apologies for our taking a while to review it! I think there are two cases where we explicitly need to create naive datetimes still to avoid comparison issues.

[tw4l]

Suggested change

	actual = datetime.now(timezone.utc) - newer
	actual = datetime.now(timezone.utc).replace(tzinfo=None) - newer

I think this is one of the few places where we do need to explicitly create a naive datetime or else an exception will be thrown due to comparison of naive and aware datetimes.

[wumpus]

newer is a relativedelta, not a datetime.

[tw4l]

Suggested change

	actual = datetime.now(timezone.utc) - older
	actual = datetime.now(timezone.utc).replace(tzinfo=None) - newer

Same as above.