sqlalchemy-bot / test_alembic_1

0 stars 0 forks source link

load_module_py & load_module_pyc do not initialize modules properly for Python3.5 #459

Closed sqlalchemy-bot closed 6 years ago

sqlalchemy-bot commented 6 years ago

Migrated issue, originally created by Abhilash Raj (maxking)

load_module_py and load_module_pyc for Python 3.5 in alembic.utils don't initialize the module properly before executing them.

This bug was found in latest release of alembic when using @public in env.py :

Traceback (most recent call last):
  File "/home/maxking/.virtualenvs/py3/bin/alembic", line 11, in <module>
    load_entry_point('alembic', 'console_scripts', 'alembic')()
  File "/home/maxking/.virtualenvs/py3/lib/python3.6/site-packages/alembic/config.py", line 479, in main
    CommandLine(prog=prog).main(argv=argv)
  File "/home/maxking/.virtualenvs/py3/lib/python3.6/site-packages/alembic/config.py", line 473, in main
    self.run_cmd(cfg, options)
  File "/home/maxking/.virtualenvs/py3/lib/python3.6/site-packages/alembic/config.py", line 456, in run_cmd
    **dict((k, getattr(options, k, None)) for k in kwarg)
  File "/home/maxking/.virtualenvs/py3/lib/python3.6/site-packages/alembic/command.py", line 465, in current
    script.run_env()
  File "/home/maxking/.virtualenvs/py3/lib/python3.6/site-packages/alembic/script/base.py", line 425, in run_env
    util.load_python_file(self.dir, 'env.py')
  File "/home/maxking/.virtualenvs/py3/lib/python3.6/site-packages/alembic/util/pyfiles.py", line 81, in load_python_file
    module = load_module_py(module_id, path)
  File "/home/maxking/.virtualenvs/py3/lib/python3.6/site-packages/alembic/util/compat.py", line 84, in load_module_py
    spec.loader.exec_module(module)
  File "<frozen importlib._bootstrap_external>", line 678, in exec_module
  File "<frozen importlib._bootstrap>", line 205, in _call_with_frames_removed
  File "/home/maxking/Documents/mm3/core/src/mailman/database/alembic/env.py", line 38, in <module>
    @public
  File "/home/maxking/.virtualenvs/py3/lib/python3.6/site-packages/atpublic-1.0-py3.6.egg/public/public.py", line 15, in public
KeyError: 'env_py'

Looks like load_module_py doesn't add the module to sys.modules which causes the error. The old load_module API did this automatically.

I am not sure if module_from_spec should take care of this (i.e. it is a bug in stdlib) or it should be addressed in alembic.

sqlalchemy-bot commented 6 years ago

Barry Warsaw (warsaw) wrote:

FWIW, I don't believe it's a bug in module_from_spec(). I think you've found the root cause of the bug.

sqlalchemy-bot commented 6 years ago

Michael Bayer (zzzeek) wrote:

There's no bug in Alembic here because env.py is treated as a single-use script, not an importable module, and the fact that it does not pollute sys.modules is by design. The implementation that uses load_module() removes the name from sys.modules immediately, so even with that version, env.py is not available to import by other parts of the application. This implemetantation was modified in #449 to work around deprecation warnings in Python 3.5 and 3.6.

I just took a look at atpublic and this library is not appropriate for an env.py script. There's no reason to place __all__ into env.py because env.py is not meant to be imported. To allow for env.py to be importable as a normal module would require some feature that allows for a user-defined module to be located by a traditional module name rather than a file location relative to the configured alembic migrations directory; it's not appropriate to use "env" as the full module name as this is too ambiguous and would conflict with other applications and Alembic migration environments trying to do the same thing. However, this feature already exists - just go into the "env.py" script, move its entire contents into whatever application module you'd like, then change env.py itself to just import that module and do nothing else.

sqlalchemy-bot commented 6 years ago

Barry Warsaw (warsaw) wrote:

Thanks for the explanation! I'll just remove the use of @public from this module.

sqlalchemy-bot commented 6 years ago

Michael Bayer (zzzeek) wrote:

Great, thanks