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

Fix quirks with import machinery in Python 3.8 #2244

Merged
merged 4 commits into from
Dec 14, 2021

Conversation

Helveg
Copy link
Contributor

@Helveg Helveg commented Dec 12, 2021

Apparently importlib.import_module sets an attribute on the parent module in python 3.8 (no longer does this in 3.9 it seems, or it was a bug that was fixed?), this didn't play nice with #2220. On top of that, the error of _setattr_error during the importlib.import_module was silenced but would lead to an AttributeError for nest.<lazy_module> even though by all means the attribute was declared, likely because _lazy_module_property uses delattr right before import_module which would then error out, dissapearing the attribute.

I also added nest.server as a lazy loaded module.

closes #2241, closes #2242, closes #2239

@babsey
Copy link
Contributor

babsey commented Dec 12, 2021

Thank you for fast fixing.

I tested nest-server on commit 6361af6 but I occur another problem to start nest-server using lazy importing.

spreizer@Latitude-5300:~$ uwsgi --module nest.server:app -s 5000
*** Starting uWSGI 2.0.19.1 (64bit) on [Sun Dec 12 16:17:46 2021] ***
compiled with version: 9.3.0 on 14 May 2021 13:30:03
os: Linux-5.11.0-41-generic #45~20.04.1-Ubuntu SMP Wed Nov 10 10:20:10 UTC 2021
nodename: Latitude-5300
machine: x86_64
clock source: unix
detected number of CPU cores: 8
current working directory: /home/spreizer
detected binary path: /usr/local/bin/uwsgi
!!! no internal routing support, rebuild with pcre support !!!
*** WARNING: you are running uWSGI without its master process manager ***
your processes number limit is 62736
your memory page size is 4096 bytes
detected max file descriptor number: 1024
lock engine: pthread robust mutexes
thunder lock: disabled (you can enable it with --thunder-lock)
uwsgi socket 0 bound to UNIX address 5000 fd 3
Python version: 3.8.10 (default, Sep 28 2021, 16:10:42)  [GCC 9.3.0]
*** Python threads support is disabled. You can enable it with --enable-threads ***
Python main interpreter initialized at 0x555603fff880
your server socket listen backlog is limited to 100 connections
your mercy for graceful operations on workers is 60 seconds
mapped 72920 bytes (71 KB) for 1 cores
*** Operational MODE: single process ***

              -- N E S T --
  Copyright (C) 2004 The NEST Initiative

 Version: HEAD@6361af614
 Built: Dec 12 2021 16:04:57

 This program is provided AS IS and comes with
 NO WARRANTY. See the file LICENSE for details.

 Problems or suggestions?
   Visit https://www.nest-simulator.org

 Type 'nest.help()' to find out more about NEST.

***
*** WARNING: NEST Server runs without a RestrictedPython trusted environment.
***
Traceback (most recent call last):
  File "<frozen importlib._bootstrap>", line 991, in _find_and_load
  File "<frozen importlib._bootstrap>", line 979, in _find_and_load_unlocked
  File "/home/spreizer/opt/nest-server/lib/python3.8/site-packages/nest/__init__.py", line 421, in _setattr_error
    return super(type(self), self).__setattr__(attr, val)
AttributeError: can't set attribute
unable to load app 0 (mountpoint='') (callable not found or import error)
*** no app loaded. going in full dynamic mode ***
*** uWSGI is running in multiple interpreter mode ***
spawned uWSGI worker 1 (and the only) (pid: 147756, cores: 1)

@Helveg: Can you test it on Python3.8? If you also reproduce the same issue, what would be your solution?

@babsey
Copy link
Contributor

babsey commented Dec 12, 2021

I confimed that the issue of nest.spatial etc. is fixed in Python 3.8.

@Helveg
Copy link
Contributor Author

Helveg commented Dec 12, 2021

Very unexpected behavior, super().__setattr__ shouldn't error out, and it doesn't in 3.9 :( I switched the implementation to use __dict__ to sidestep this, but doing so we stray 1 step further from the light ;p It should be fixed now

@babsey
Copy link
Contributor

babsey commented Dec 12, 2021

Great! NEST Server is working again. 👍 🥳

I am confident with this pull instead of #2239.
Thank you @Helveg!

Copy link
Contributor

@babsey babsey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@jougs jougs self-requested a review December 13, 2021 20:47
@jougs jougs added I: Internal API Changes were introduced in basic internal workings of the simulator that developers need to know S: Critical Needs to be addressed immediately T: Bug Wrong statements in the code or documentation labels Dec 13, 2021
@jougs jougs added this to In progress in PyNEST via automation Dec 13, 2021
@jougs jougs added this to the NEST 3.2 milestone Dec 13, 2021
Copy link
Contributor

@jougs jougs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks pretty clean to me. Many thanks for addressing the issue. I also tested that

mpirun --oversubscribe -np 4 install/bin/nest-server-mpi

works and it does. Thus I'm approving.

pynest/nest/__init__.py Outdated Show resolved Hide resolved
PyNEST automation moved this from In progress to Review Dec 13, 2021
@jougs
Copy link
Contributor

jougs commented Dec 13, 2021

@terhorstd: Given the criticality of this, can you please also have a quick look? Thanks!

Co-authored-by: Jochen Martin Eppler <jougs@gmx.net>
@jougs
Copy link
Contributor

jougs commented Dec 14, 2021

@steffengraber: I will merge this now. Can you please comment here, if there are any problems on Read the Docs caused by the merge? Thanks!

@jougs jougs changed the title Fixed quirks with import machinery in python 3.8 Fix quirks with import machinery in Python 3.8 Dec 14, 2021
@jougs jougs merged commit dc31ac6 into nest:master Dec 14, 2021
PyNEST automation moved this from Review to Done Dec 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I: Internal API Changes were introduced in basic internal workings of the simulator that developers need to know S: Critical Needs to be addressed immediately T: Bug Wrong statements in the code or documentation
Projects
PyNEST
  
Done
Development

Successfully merging this pull request may close these issues.

Start uwsgi with nest.server failed Lazy loading modules for PyNEST
3 participants