alembic icon indicating copy to clipboard operation
alembic copied to clipboard

add a classification to commands that they don't need a database, e.g. a third option to "online" / "offline" mode

Open mattinbits opened this issue 6 years ago • 10 comments

I am having an issue running alembic history. Because I have migrations which rely on modules within my project, I get a ModuleNotFoundError when running alembic history. I can work around this by setting the python path but I would rather the command was working "out of the box".

I tried setting revision_environment to true in the ini file, along with correcting the path in env.py. This results in an error caused by attempting to connect to the database, since the details required to connect are not available.

Is there an existing approach to being able to run alembic history with the revision environment but without the connection? If not would it make sense to alter the behaviour of history to load the environment in offline mode except where required (e..g when executed with --indicate_current)

mattinbits avatar Nov 04 '19 10:11 mattinbits

FWIW, I see the same with alembic revision too.

mattinbits avatar Nov 04 '19 11:11 mattinbits

for history this is issue #447 and for both revision and history you need to set revision_environment to True: https://alembic.sqlalchemy.org/en/latest/tutorial.html?highlight=revision_environment#editing-the-ini-file the doc in the .ini file needs to specify this works for history also and there should be a disitnct doc section for this flag.

zzzeek avatar Nov 04 '19 13:11 zzzeek

Hi, thanks for the quick response. I did try this setting and while it does prevent the ModuleNotFoundError, it leads to a different error since env.py attempts to connect to the database but is not able to in the context I'm running it (I think this means it executes the environment in "online" mode). The crux of my question is, is this an expected limitation? Or would it be appropriate for history and revision to run in offline mode except where they are executed with flags which imply connectivity is necessary. I'd be prepared to have a go at a PR if you consider it a reasonable solution.

mattinbits avatar Nov 04 '19 13:11 mattinbits

Well env.py is where you can put module imports, so that's the file we're doing.

if you need it to skip doing the connection, you should modify env.py to detect this condition (when DB connection is not expected to be present) and then skip trying to connect.

the env.py file that's given already has some of this going on with the "context.is_offline_mode()" check. At the moment, it's not that straightforward to get the direct name of the command running, like "if command == 'history'", however I could show you how to do that for now and we could document it (you'd look in context.cmd_opts, I think). I'd be happy to support adding a third boolean "context.non_database_command()" to distinguish commands that the database is not relevant. though for revision, it is relevant if you have --autogenerate.

im answering this quickly so need to think about it more but short answer is modify env.py to do what you want it to do.

zzzeek avatar Nov 04 '19 13:11 zzzeek

my hesitancy with using "is_offline_mode" is that this would impact existing env.py scripts and would need to think if there are any caveats to doing this.

zzzeek avatar Nov 04 '19 13:11 zzzeek

Thanks for the pointer towards context.cmd_opts. From this I was able to do the following in my env.py:

cmd_name = config.cmd_opts.cmd[0].__name__

if cmd_name == "history" and not config.cmd_opts.indicate_current:
    run_command_offline = True
else:
    run_command_offline = False

if context.is_offline_mode() or run_command_offline:
    run_migrations_offline()
else:
    run_migrations_online()

I also had to update my run_migrations_offline function to add `as_sql=True:

context.configure(
        url=url,
        target_metadata=target_metadata,
        literal_binds=True,
        as_sql=True,
        dialect_opts={"paramstyle": "named"},
    )

This is now working nearly exactly as I want except that I get additional output that pollutes the history output such as BEGIN TRANSACTION; (completely expected since the configuration is to print the SQL.)

However I looked at the commentary on https://github.com/sqlalchemy/alembic/issues/460 and I agree with your conclusion that this is something that should be solved with PYTHONPATH. I've realized I can configure my python path per virtualenv following https://stackoverflow.com/a/4758351/5142537

So I don't think I'll pursue the option of solving it programmatically in env.py but leaving it here for anyone else who finds it useful.

mattinbits avatar Nov 04 '19 15:11 mattinbits

(Happy for this to be closed unless you would like to pursue it further)

mattinbits avatar Nov 04 '19 15:11 mattinbits

this is an area that should be improved, I just noticed that "alembic revision" accepts --sql to avoid connecting to the DB but it appears the flag is mis-documented.

I think there should be an overall flag that "this command doesn't normally need the database" that can be detected in env.py through a new function in the template so that we can more easily have env.py take part in commands.

zzzeek avatar Nov 04 '19 16:11 zzzeek

Mike Bayer has proposed a fix for this issue in the master branch:

Use looser tokenization for type comparison https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/1802

sqla-tester avatar Mar 19 '20 16:03 sqla-tester

this was inadvertently closed as I put the wrong changeset number on the above review

zzzeek avatar Mar 19 '20 22:03 zzzeek