[Django] #36633: TaskBackend run_after validation assumes datetime when USE_TZ=True

[Django]

#36633: TaskBackend run_after validation assumes datetime when USE_TZ=True
-------------------------+-----------------------------------------
Reporter: Chris M | Type: Uncategorized
Status: new | Component: Tasks
Version: 6.0 | Severity: Normal
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------+-----------------------------------------
I’ve been working on an implementation of a TaskBackend using Google Cloud
Tasks. I ran across a bit of confusion regarding the {{{run_after}}}
parameter validation.

The DEP originally referenced that the value could be a datetime or a
timedelta. The released implementation looks like it only documents it as
a datetime, so I’m not sure if it was meant to support timedelta. However,
I tried using a timedelta with the {{{USE_TZ=True}}} and the validation in
BaseTaskBackend assumes that the run_after value is a {{{datetime}}} and I
got an error

Calling code:
{{{my_task.using(run_after=timedelta(seconds=10)).enqueue()}}}

Error: {{{AttributeError: 'datetime.timedelta' object has no attribute
'utcoffset'}}}

If I run the code using a {{{timedelta}}} and with {{{USE_TZ=False}}} then
the error isn't thrown and my backend can process it.

The code throwing the error in the {{{BaseTaskBackend}}} is

{{{#!python
if (
settings.USE_TZ
and task.run_after is not None
and not timezone.is_aware(task.run_after)
):
raise InvalidTask("run_after must be an aware datetime.")
}}}

I think it would be best for this condition to verify that the provided
{{{run_after}}} is a {{{datetime}}} before calling
{{{timezone.is_aware}}}. I'm hoping for some confirmation if this is the
right way to proceed.

I think it would be great to support {{{timedelta}}} directly as well by
updating the type annotations on {{{Task}}}, but I couldn't determine
through the PR history if this was removed during implementation.
--
Ticket URL: <https://code.djangoproject.com/ticket/36633>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36633: TaskBackend run_after validation assumes datetime when USE_TZ=True

---------------------------------+--------------------------------------
Reporter: Chris M | Owner: (none)
Type: Bug | Status: new
Component: Tasks | Version: 6.0
Severity: Release blocker | Resolution:

Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

---------------------------------+--------------------------------------
Changes (by Jacob Walls):

* cc: Jake Howard (added)
* severity: Normal => Release blocker
* type: Uncategorized => Bug

--
Ticket URL: <https://code.djangoproject.com/ticket/36633#comment:1>

[Django]

#36633: TaskBackend run_after validation assumes datetime when USE_TZ=True
---------------------------------+------------------------------------

Reporter: Chris M | Owner: (none)
Type: Bug | Status: new
Component: Tasks | Version: 6.0
Severity: Release blocker | Resolution:

Keywords: | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------

Changes (by Mariusz Felisiak):

* stage: Unreviewed => Accepted

--
Ticket URL: <https://code.djangoproject.com/ticket/36633#comment:2>

[Django]

#36633: TaskBackend run_after validation assumes datetime when USE_TZ=True
---------------------------------+------------------------------------
Reporter: Chris M | Owner: (none)

Type: Bug | Status: closed
Component: Tasks | Version: 6.0
Severity: Release blocker | Resolution: wontfix

Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------

Changes (by Jake Howard):

* resolution: => wontfix
* status: new => closed

Comment:

I believe this is behaving as intended. The `timedelta` functionality was
removed post-DEP, because when interacting with the previous transaction
handling, it wasn't obvious if it was "10 seconds after enqueue" or "10
seconds from `.enqueue` call". We could add a type guard that only a
`datetime` is passed in, but given an exception is already raised, rather
than strange behaviour, I suspect it's fine.
--
Ticket URL: <https://code.djangoproject.com/ticket/36633#comment:3>

[Django]

#36633: TaskBackend run_after validation assumes datetime when USE_TZ=True

---------------------------------+--------------------------------------

Reporter: Chris M | Owner: (none)
Type: Bug | Status: closed
Component: Tasks | Version: 6.0
Severity: Release blocker | Resolution: wontfix

Keywords: | Triage Stage: Unreviewed

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

---------------------------------+--------------------------------------
Changes (by Mariusz Felisiak):

* stage: Accepted => Unreviewed

--
Ticket URL: <https://code.djangoproject.com/ticket/36633#comment:4>

[Django]

#36633: TaskBackend run_after validation assumes datetime when USE_TZ=True
---------------------------------+--------------------------------------
Reporter: Chris M | Owner: (none)
Type: Bug | Status: closed
Component: Tasks | Version: 6.0
Severity: Release blocker | Resolution: wontfix
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+--------------------------------------

Comment (by Chris M):

Thanks for the context. I figured there was probably a conversation about
the `timedelta` that I was just missing. Personally, I think it is more
confusing to have the time be static, when the transaction close time is
dynamic, but I understand needing to make a call and go with it, not a
problem.

If you are open to it, Jake, I'm happy to cut a PR against `django-tasks`
for the type check before the timezone aware check. You are correct that
it is failing, which is the final outcome either way. However, as a
developer interacting with it, I was confused about what steps I should
take, since the error had less to do with making the value timezone aware
and more that I just passed in the wrong type completely. I think the
clean up here could create a better developer experience.
--
Ticket URL: <https://code.djangoproject.com/ticket/36633#comment:5>