Skip to content
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

'async' are reserved keywords in python 3.7 #25578

Closed
ZOOUQinn opened this issue Jul 3, 2018 · 9 comments
Closed

'async' are reserved keywords in python 3.7 #25578

ZOOUQinn opened this issue Jul 3, 2018 · 9 comments
Labels

Comments

@ZOOUQinn
Copy link

ZOOUQinn commented Jul 3, 2018

According document of 'What’s New In Python 3.7', 'async' are reserved keywords now.

Code in line 105 of 'odoo/addons/base/ir/ir_qweb/assetsbundle.py'
def to_html(self, sep=None, css=True, js=True, debug=False, async=False, url_for=(lambda url: url)):
will throw SyntaxError out if we launch odoo with this update python.

@pedrobaeza
Copy link
Collaborator

Good point, although that version is not officially supported yet, it's good to check for possible problems.

@mart-e who can handle this?

@mart-e
Copy link
Contributor

mart-e commented Jul 3, 2018

will throw SyntaxError out if we launch odoo with this update python

Are you sure about it? Won't you just be able to use the python async from this file?
Same as you can do

def foo(float=42):
     ....

even if this is a bad idea

@pedrobaeza
Copy link
Collaborator

In other versions, using reserved keywords, you redefine its meaning for that context, but no error is raised

@guewen
Copy link
Contributor

guewen commented Jul 3, 2018

@mart-e float is a builtin type not a keyword, async is a keyword starting from python 3.7, like def or class are. You can't do that:

def foo(class=42):
    ....

It's the same with async now.

@mart-e
Copy link
Contributor

mart-e commented Jul 3, 2018

@guewen oh right, thanks for the clarification. That will be problematic as changing that would change the signature which may break the compatibility in stable versions.
We have to find a workaround to avoid forcing a maximum version of python for v11...

@yvaucher
Copy link
Contributor

yvaucher commented Jul 3, 2018

@mart-e maybe not the cleanest way but for retro compatibility, you could rename the key argument in the signature plus adding kwargs and then you search if kwargs contains an 'async' key. That way you only work with string for retro compatibility and the issue is on the caller side.

@Yenthe666 Yenthe666 added the 11.0 label Jul 5, 2018
@dungnm8

This comment has been minimized.

@hbto
Copy link
Contributor

hbto commented Jul 16, 2018

Hello guys, I will take a quote from here

Freezing Your Code

As version 11 of odoo came first than the python3.7, and making workarounds would render odoo useless for others using the original signature of the methods that involved with keywords reservation of Python3.7.

In my opinion, it is better to suggest frozen requirement on python version stating it works with 3.6
and make the changes in master/post-11 version.

Edit:
Reading a little more async and await usage dates back to Python 3.5 / September 13, 2015 which then were made reserved keywords on 3.7.

Yet I keep my suggestion, Freezing versions of requirements.

Regards.

mart-e added a commit to odoo-dev/odoo that referenced this issue Jul 17, 2018
In python 3.7, async is a reserved keyword.
Using it in a method signature produces a syntax error.
Replace by an argument async_load and catch the previous calls in a kw for
backward compatibility.
Log a warning on public methods.

The **kw can be removed in v12 and officially deprecate the 'async' parameter.

Fixes odoo#25578
Closes odoo#25783
@mart-e
Copy link
Contributor

mart-e commented Jul 17, 2018

The compatibility to python 3.7 has been fixed in the following commits:
af9d6b8 [ADD] tools: add new 3.7 opcodes
5a57c29 [FIX] base: do not use async reserved keyword

Freezing could be an option but is a radical change in the way we ship Odoo. If we can maintain the compatibility with several versions, it is better.
Python 3.7 will soon be shipped on different linux distributions (afaik not yet the case) and it would make it a lot harder to deploy Odoo.

moylop260 added a commit to vauxoo-dev/odoo that referenced this issue Jul 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

8 participants