Pyright Task bug

[mecampbellsoup]

Hello!

Here is our task definition:

@task_queue.task(**send_outbound_email_task_args)
async def _send_email_task(context: JobContext, email_data: EmailData):
    # HubSpot will not send a duplicate email with the same sendId. We need to
    # construct a string that is always different between dev/test/staging/prod,
    # and that will allow job retries. When a failed job is retried, the same
    # job.id is used and the number of attempts are tracked. So job.id is a
    # reliable unique identifier for an email within any cloud-app deployment.
    # This allows for multiple emails with the same content to be sent even if
    # different jobs are created.
    pass

And here is our dependency injection pattern. The task is called/invoked in DeferredEmailSender:

class EmailSender(ABC):
    @abstractmethod
    def send(self, email: BaseOutboundEmail) -> None:
        ...

class DeferredEmailSender(EmailSender):
    def send(self, email: BaseOutboundEmail) -> None:
        _send_email_task.defer(email_data=email.email_data)

When I try to commit this, pyright complains:

pyright..................................................................Failed
- hook id: pyright
- duration: 1.98s
- exit code: 1

WARNING: there is a new pyright version available (v1.1.363 -> v1.1.369).
Please install the new version or set PYRIGHT_PYTHON_FORCE_VERSION to `latest`

/home/mcampbell/code/coreweave/cloud-console/cloud_console/core/emails/sender.py
  /home/mcampbell/code/coreweave/cloud-console/cloud_console/core/emails/sender.py:28:26 - error: Cannot access attribute "defer" for class "function"
    Attribute "defer" is unknown (reportFunctionMemberAccess)
1 error, 0 warnings, 0 informations

[INFO] Restored changes from /home/mcampbell/.cache/pre-commit/patch1719599246-864754.

As far as I can tell we are using "vanilla" procrastinate syntax here. How can I make pyright happy again? Thanks!

[ewjoachim]

Looks like you're using pyright v1.1.363.

Procrastinate currently uses v1.1.357 for its own checks, and we've already witnessed that we need to change things to support v1.1.368.

We recently greatly improved the typing in #1061 and I believe we still have some work to do to get it working. If it's a subject that interest you, you're very welcome to give a hand, either in identifying what's wrong or even if you want to try and fix it. Otherwise, it might be a priority, but I can't commit on a specific timeframe.

[ewjoachim]

(@onlyann you might be interested by this :) )

[onlyann]

@mecampbellsoup what happens if you specify explicitly pass_context=True in the task decorator options?

EDIT:

In your code extract, by packing**send_outbound_email_task_args, pyright cannot match it against one of the valid task overloads that returns a decorator. As far as pyright is concerned, that function is not decorated.

The typing of @task has been crafted to identify when pass_context=True is provided or not. When it is provided, the decorated function needs a first argument of type JobContext. 

I have been able to replicate the issue with the latest version of pyright. I could fix it by amending the code slightly:

@task_queue.task(**send_outbound_email_task_args, pass_context=True)
async def _send_email_task(context: JobContext, email_data: EmailData):

Notice the extra pass_context=True I added at the end. This way, pyright can correctly match the decorator signature.

[ewjoachim]

(pass_context rather than passContext, but :+1: for the rest)

[mecampbellsoup]

Wow great response guys thank you!

[onlyann]

Wow great response guys thank you!

You're welcome. Feel free to close the issue if that solves your case.

[mecampbellsoup]

@onlyann quick follow up...

I changed our Python code to:

retry = RetryStrategy(exponential_wait=2)
send_outbound_email_task_kwargs = {"retry": retry, "queue": "outbound_email"}

# TODO: @sentry.suppress_sentry_before_retries(cloud_console.settings.MIN_RETRIES_BEFORE_SENTRY)
# TODO: @sentry.label_job_exceptions
# NOTE: The `pass_context` kwarg is passed explicitly for Pyright.
# See https://github.com/procrastinate-org/procrastinate/issues/1087
@task_queue.task(**send_outbound_email_task_kwargs, pass_context=True)
async def _send_email_task(context: JobContext, email_data: EmailData):
    ...

Now, pyright is complaining about a test I wrote that calls _send_email_task directly:

@patch("cloud_console.core.emails.sender.APP_NAME", new="cloud-console")
@patch("cloud_console.core.emails.sender.hubspot_client.send_email")
async def test_send_email_task(mock_send_email):
    job_id = 12345
    mock_context = Mock()
    mock_context.job.id = job_id
    email_data = EmailData(
        {
            "emailId": 46404925597,
            "message": {"to": "test@example.com", "from": CW_SUPPORT_EMAIL},
            "customProperties": {},
        }
    )
    await _send_email_task(context=mock_context, email_data=email_data)

pyright..................................................................Failed
- hook id: pyright
- duration: 5.65s
- exit code: 1

WARNING: there is a new pyright version available (v1.1.363 -> v1.1.369).
Please install the new version or set PYRIGHT_PYTHON_FORCE_VERSION to `latest`

/home/mcampbell/code/coreweave/cloud-console/tests/core/emails/test_send_email.py
  /home/mcampbell/code/coreweave/cloud-console/tests/core/emails/test_send_email.py:29:36 - error: Expected 1 more positional argument (reportCallIssue)
1 error, 0 warnings, 0 informations

[onlyann]

@mecampbellsoup JobContext is a positional argument, not a keyword argument.

Instead of:

_send_email_task(context=mock_context, email_data=email_data)

Use:

_send_email_task(mock_context, email_data=email_data)

[ewjoachim]

If you want to make it extra clear when defining your task that you expect the job context to be a positional argument, you could define it like:

async def _send_email_task(context: JobContext, /, *, email_data: EmailData):

(everything before the / must be positional. Everything after the * must be named. Everything between / and * may be either)

This way you'd not only get typing error but also test crash if you're not calling the task the way procrastinate will call it.