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

Format checking/fixing script breaks with python3.8 #12740

Closed
LisaLudique opened this issue Aug 19, 2020 · 3 comments · Fixed by #12743
Closed

Format checking/fixing script breaks with python3.8 #12740

LisaLudique opened this issue Aug 19, 2020 · 3 comments · Fixed by #12743

Comments

@LisaLudique
Copy link
Contributor

LisaLudique commented Aug 19, 2020

Description:

Running tools/code_format/check_format.py check (or with fix) with python3.8.0 or python3.8.7 on MacOS fails with the following error:

➜  envoy git:(master) tools/code_format/check_format.py check
multiprocessing.pool.RemoteTraceback: 
"""
Traceback (most recent call last):
  File "/Users/lisalu/.pyenv/versions/3.8.0/lib/python3.8/multiprocessing/pool.py", line 125, in worker
    result = (True, func(*args, **kwds))
  File "/Users/lisalu/src/envoy/tools/code_format/check_format.py", line 965, in checkApiShadowStarlarkFiles
    if operation_type == "check":
NameError: name 'operation_type' is not defined
"""

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "tools/code_format/check_format.py", line 1152, in <module>
    error_messages += sum((r.get() for r in results), [])
  File "tools/code_format/check_format.py", line 1152, in <genexpr>
    error_messages += sum((r.get() for r in results), [])
  File "/Users/lisalu/.pyenv/versions/3.8.0/lib/python3.8/multiprocessing/pool.py", line 768, in get
    raise self._value
NameError: name 'operation_type' is not defined

While the argument is correctly parsed into the operation_type variable in the main function it is undefined/not visible to the workers running checkFormat. I am guessing this behavior changed with Python3.8 (https://bugs.python.org/issue39931?) as I am able to run the script successfully with Python3.7.8 and older versions.

@LisaLudique
Copy link
Contributor Author

This applies to some of the other command line arguments like build_fixer_check_excluded_paths; I am working on a PR to pass those variables into the helper functions that use them.

@stale
Copy link

stale bot commented Sep 20, 2020

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or other activity occurs. Thank you for your contributions.

@stale stale bot added the stale stalebot believes this issue/PR has not been touched recently label Sep 20, 2020
@jingwei99
Copy link
Contributor

I am running the same issue, and the fix from #12743 works for me, it'll be great for it to have it merged 🙌

@stale stale bot removed the stale stalebot believes this issue/PR has not been touched recently label Sep 22, 2020
htuch pushed a commit that referenced this issue Sep 23, 2020
)

The formatting script does not recognize the variables from the command line arguments when running Python >=3.8 on MacOS due to changes in multiprocessing behavior in the new Python version (https://bugs.python.org/issue39931). This change passes in the command line arguments so they are accessible by the helper functions in which they are needed.

Risk Level: Low
Testing: Existing unit tests
Docs Changes: N/A
Release Notes: N/A

Fixes #12740

Signed-off-by: Lisa Lu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants