Skip to content

using async in names is deprecated on python 3.6 #21

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

johnnoone
Copy link

No description provided.

@ghost
Copy link

ghost commented Jul 25, 2016

It looks like @johnnoone hasn't signed our Contributor License Agreement, yet.

Appreciation of efforts,

clabot

@edenhill edenhill assigned benstopford and unassigned benstopford Jul 26, 2016
@ewencp
Copy link
Contributor

ewencp commented Jul 29, 2016

@johnnoone Good catch, we would like to avoid abusing the keyword if possible. @edenhill I think the main concern here is probably compatibility. Are we comfortable breaking the existing API given how new this is? Or should we make the patch support both async and asynchronous for now, deprecate async, and remove it in a later release?

@edenhill
Copy link
Contributor

edenhill commented Aug 4, 2016

This is still a young project so I vote for not being backwards compatible, let's go with asynchronous in the next release.

Is asynchronous user friendly enough, or should it be something easier to spell, like do_async?

@ewencp
Copy link
Contributor

ewencp commented Aug 4, 2016

@edenhill Doesn't look like the PEP suggests a specific alternative. Maybe check a couple of other libraries that have async options to see if there is a de facto standard?

@drice drice mentioned this pull request Aug 4, 2016
@edenhill
Copy link
Contributor

edenhill commented Dec 7, 2016

Sort of missed the train with this one, so we'll add asynchronous=bool kwarg but keep the old async=bool one for backwards compatibility (possibly with a warning), e.g. something like:

 def foo (.., asynchronous=False, async=None):
    if async is not None:
         warning('Use asynchronous instead, kthx')
         asynchronous=async
...

@edenhill edenhill added this to the v0.9.3 milestone Jan 20, 2017
@ah-
Copy link

ah- commented Feb 19, 2018

I think it's time to implement this now, since async has started to error on Python 3.7 nightly:

E       self.consumer.commit(offsets=offsets_to_commit, async=False)
E                                                           ^
E   SyntaxError: invalid syntax

Renaming async to asynchronous sounds like a decent solution.

@edenhill
Copy link
Contributor

@ah- I think we should provide an "async" alias for <Py3.7 for a while to not break existing applications. What do you think?

We have an upcoming 1.0.0 release bump coming up in a couple of months at which time we can break the API.

@ah-
Copy link

ah- commented Feb 19, 2018

Yes, keeping backwards compatibility would be great. Looking at https://github.com/confluentinc/confluent-kafka-python/blob/master/confluent_kafka/src/Consumer.c#L378 it should be as trivial as adding "asynchronous" to the list of kwargs and then having async = PyObject_IsTrue(async_o) || PyObject_IsTrue(asynchronous_o).

We can probably even keep the old async afterwards, as far as I can tell the only thing that breaks is writing async=False, you could still pass kwargs manually setting {'async': True}.
So no breakage required, just an alias so Py 3.7 users can use normal syntax to call.

@lithammer
Copy link

Something like this ought to do it (not very pretty though!):

def foo(..., asynchronous=None, **kwargs):
    asynch = asynchronous or kwargs.get('async')

@edenhill
Copy link
Contributor

@Renstrom That will allow any (unknown) kw arg to be set though. Say Hi to Löfling from me :)

A PR is on the way.

@edenhill
Copy link
Contributor

Updated PR in #319

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants