Rq: Python 3.7 compatibility

[mattrobenolt]

In quick testing, there's at least one issue with 3.7 and rq:

https://github.com/rq/rq/blob/master/rq/queue.py#L61

async will become a reserved word and this results in a SyntaxError.

[samuelcolvin]

pull request welcome :smile:

[mattrobenolt]

Yup, I probably will. :)

Do you have a preference for how to do this? It'll be an API change since we can't use async as a kwarg anymore. What I was thinking is this:

• Replace with sync and make this work like the exact opposite of async. So the default just being sync=False. This is a breaking API change and would probably require a 0.11 bump? Not sure what your versioning scheme is.
• We can try to maintain backwards compat by accepting **kwargs then checking for kwargs.get('async').

I'd prefer not to maintain the backwards compat, but this might be a common enough use case for people that it'd be better to? Your call. I'm fine with implementing either and throwing it up as PR.

In the meantime, not sure if there are any other 3.7 issues, since I couldn't even get it to run due to this being a SyntaxError, but I don't anticipate there being other issues.

[samuelcolvin]

Up to @selwin really but I would say we're best to just change the API, perhaps with a catch for "async" kwarg that gives a helpful error message?

[mattrobenolt]

Yeah, we can do something like:

if 'async' in kwargs:
    raise ValueError('use sync now')

Or whatever is preferred. We just can't leave it around as-is without a SyntaxError.

[selwin]

Agree with @samuelcolvin:

1. Change async to sync
2. Catch async in kwargs and give a helpful error message.

We can then release version 1.0 which is backwards incompatible with earlier versions. With this version, I want to also convert FailedQueue to FailedJobRegistry which scales a lot better to a large number of jobs.

[mattrobenolt]

Cool, I'll poke at this tonight probably and see if there are any other issues with python 3.7 after this is fixed. I'll make sure the test suite all passes.

[theodesp]

@mattrobenolt any progress on this?

[mattrobenolt]

I have not yet, unfortunately.

[mikob]

I've not done "2. catch async in kwargs and give a helpful error message" but is this the right direction so far?

[mattrobenolt]

I suggested that just for backwards compatibility since this strictly is breaking the API and also with a value that is not the exact opposite meaning. There's no reason you _have_ to keep this, it's just a nice to have I think for people who are upgrading.

[samuelcolvin]

I made a comment on the PR, but perhaps better to comment here:

Surely it would be clearer to use asynchronous or async_ to keep things the same rather than switching from async=True to sync=False?

[selwin]

Yeah, let's go with asynchronous=True. PR welcome :)

[Flimm]

Another suggestion is asynk. Many Python programmers are already used to typing klass in Python class methods instead of the reserved word class, and asynk is much shorter than asynchronous.

[selwin]

@Flimm decided to go with sync as the keyword argument. I don't like the aesthetics of async_.

Besides the slightly harder code migration path where you have to reverse the arguments, If I were to make RQ from scratch today, I would have chosen to use sync as the keyword. See the discussion here: https://github.com/rq/rq/pull/963#issuecomment-400719813

[samuelcolvin]

Sounds good.

[Flimm]

Thanks @selwin . I think there's a typo in your comment, I think you meant to say:

@Flimm decided to go with asynk as the keyword argument.

Also, note that this is talking about a pull request that hasn't been accepted yet. I don't think this issue should be closed until the pull request #971 is accepted.

[selwin]

@Flimm sorry, what I meant is that @Flimm *I* decided to go with sync. @mikob made a PR to https://github.com/rq/rq/pull/970 to change the keyword argument to sync and this was already merged to the 1.0 branch :)

[Flimm]

Ah, I see! Thanks @selwin .

For the lurkers, even though this issue is closed, the current latest release of rq on PyPI (0.11) is not compatible with Python 3.7.