sqlalchemy-bot / test_alembic_1

0 stars 0 forks source link

add environment control for *all* ScriptDirectory commands #460

Open sqlalchemy-bot opened 7 years ago

sqlalchemy-bot commented 7 years ago

Migrated issue, originally created by Mischa (mischas)

continuing from #447:

all commands that use ScriptDirectory.from_config() should be altered to use a context manager that examines "revision_environment" and then optionally makes use of EnvironmentContext.

the idea is that people are now coding imports into their revision files that need to be pulled in from env.py in order for the files to be importable. ScriptDirectory is not usable without this directive.

sqlalchemy-bot commented 7 years ago

Michael Bayer (zzzeek) wrote:

there are many commands that look at the script directory yet do not use the environment. this issue suggests that all of them need to be modified. what is the use case that you need the environment to run in order to run "heads" ?

sqlalchemy-bot commented 7 years ago

Mischa (mischas) wrote:

I import some information used in a few migrations, for example defining Enums centrally. The migrations file can resolve the imports fine when env.py is loaded, but can't when running "heads".

sqlalchemy-bot commented 7 years ago

Michael Bayer (zzzeek) wrote:

I'm actually not following that. What are the imports in your version files exactly and what code is inside of "env.py" that make this possible? or are you importing these objects as "from env import some_enum"? that's not supported.

sqlalchemy-bot commented 7 years ago

Michael Bayer (zzzeek) wrote:

in #447 the reporter is just setting PYTHONPATH in env.py, and actually, that's not ideal. Running the whole env just to set PYTHONPATH seems like overkill.

sqlalchemy-bot commented 7 years ago

Mischa (mischas) wrote:

I'm importing an Enum from my code so I can use the types it defines as constants. I'm not importing from env. PYTHONPATH might work for me

sqlalchemy-bot commented 7 years ago

Michael Bayer (zzzeek) wrote:

I think I've come up with an approach that might work out, which is add a new hook in env.py that's specifically for "non-database, non-SQL" commands. if this hook is present then we use the env.py, but I think this means we might have to import env.py no matter what. have to think about it more.

However I still have no idea how your specific case works. your description at the top says, "people are now coding imports into their revision files that need to be pulled in from env.py in order for the files to be importable", but then you just said, " I'm not importing from env.". these seem to contradict?

sqlalchemy-bot commented 7 years ago

Mischa (mischas) wrote:

I thought you were asking if I was actually importing from "env", which I'm not. So I have a flask app and it requires a small bit of setup for the DB stuff, meaning a method has to be called before my model files are available and loaded. So the migrations try to load a model file which contains the Enum and fails because my setup() method hasn't been called. It's kind of lame I know.

sqlalchemy-bot commented 7 years ago

Michael Bayer (zzzeek) wrote:

perhaps a new file in the script area called "init_revisions.py", runs unconditionally for every command no matter what. starts out as blank.

sqlalchemy-bot commented 7 years ago

Mischa (mischas) wrote:

I guess from my unwashed idiot user perspective I don't understand why env.py is run for "migrate" but not for "heads". Is there a conceptual difference I should be aware of? They both involve the revisions files.

sqlalchemy-bot commented 7 years ago

Michael Bayer (zzzeek) wrote:

or maybe something totally different, we don't actually import the migration files when we need to do simple things like "heads".

sqlalchemy-bot commented 7 years ago

Michael Bayer (zzzeek) wrote:

the conceptual difference is that the original point of "env.py" was, "set up the database connection and connect to it", as well as, "set up options and flags and stuff for complicated operations like upgrades, downgardes, autogenerate". a command like "heads" only needs to show a few files in the scripts directory.

running env.py in all cases is doable but would need some refactoring to make it palatable for every command. it also makes the fate of "revision_environment" unclear. Considering I keep missing the mark everytime I have to fix something in this area I need to be very careful.

sqlalchemy-bot commented 6 years ago

Mischa (mischas) wrote:

It would be enough for now to just set PYTHONPATH. Right now I have to do this to run commands: PYTHONPATH=. alembic heads

sqlalchemy-bot commented 6 years ago

TMiguelT (TMiguelT) wrote:

I believe I'm encountering this issue with standard the alembic revision command, which doesn't seem to use env.py unless you use the --autogenerate flag. This StackOverflow question summarises the issue, and the answers include some of the workarounds that people are using: https://stackoverflow.com/questions/15648284/alembic-alembic-revision-says-import-error/15769215

sqlalchemy-bot commented 6 years ago

Michael Bayer (zzzeek) wrote:

the way folks in my universe do things is: we make our application into a real installable app, we have a setup.py, then we make sure we are always running from an environment where our app is "installed", if that is even a local virtual environment where we installed with "pip install -e .". I guess that approach is getting lost to the sands of time, or something, it used to be how everybody did everything.

sqlalchemy-bot commented 6 years ago

Michael Bayer (zzzeek) wrote:

@tmiguelt please note if you are only dealing with "alembic revision", you can use env.py every time, use revision_environment=True, it's right in your alembic.ini file and is documented

http://alembic.zzzcomputing.com/en/latest/tutorial.html?highlight=revision_environment#editing-the-ini-file

#!

# set to 'true' to run the environment during
# the 'revision' command, regardless of autogenerate
# revision_environment = false
sqlalchemy-bot commented 6 years ago

Michael Bayer (zzzeek) wrote:

also that stackoverflow user seems to be having a different problem, their actual env.py doesn't run without a pythonpath setup. this points towards two things:

  1. add a real section to the alembic documentation that suggests the standard route of using a setup.py w/ virtualenv

  2. add the init_revisions.py script for any custom path setup stuff, having it separate from env.py means env.py can continue to have path-specific imports at the top. maybe script should be setup_python_env.py or something.

sqlalchemy-bot commented 6 years ago

Mischa (mischas) wrote:

@zzzeek I usually do something like: this

Maybe that's stupid. I don't know.

sqlalchemy-bot commented 6 years ago

Michael Bayer (zzzeek) wrote:

I'm searching yet again for this immensely common pattern and still not finding good links that combine web app + setup.py + virtualenv all at once, so weird.

sqlalchemy-bot commented 6 years ago

Michael Bayer (zzzeek) wrote:

@mischas yeah that's the pattern that building yourself a virtualenv is meant to solve

sqlalchemy-bot commented 6 years ago

TMiguelT (TMiguelT) wrote:

we make our application into a real installable app

That does solve this problem, but it's not always a common approach for applications, because it means you need to:

if you are only dealing with "alembic revision", you can use env.py every time, use revision_environment=True

Sorry yes you're right, I didn't realise this was here. I'm sure this will solve my issue! (it will let me load my sys.path hacks)

add the init_revisions.py script for any custom path setup stuff, having it separate from env.py means env.py can continue to have path-specific imports at the top. maybe script should be setup_python_env.py or something.

I'm not entirely sure what you mean by this, but I would advise against splitting out the env.py configuration for the sake of simplicity. If this problem is already solved by what we've discussed then it's probably not too much of an issue.


In general, it seems that there are 3 ways to solve this path issue:

Thinking about it now, I do prefer this last option, as you've said, so maybe suggesting that in the documentation isn't a bad idea

sqlalchemy-bot commented 6 years ago

Mischa (mischas) wrote:

@zzzeek I use a virtualenv for dependencies of course. In that specific example I have a 'vendor' dir for deploying to AWS lambda though. I don't understand why pip install . though? Every time you make a change? How does that work with dev server reloading?

sqlalchemy-bot commented 6 years ago

Mischa (mischas) wrote:

My path hack doesn't help with alembic at all if I'm just trying to run commands like "heads", "merge", etc. I still have to do export PYTHONPATH=. before running alembic.

sqlalchemy-bot commented 6 years ago

TMiguelT (TMiguelT) wrote:

I don't understand why pip install . though? Every time you make a change?

If you pip install -e ., it makes an editable install which solves most of these issues: https://pip.pypa.io/en/stable/reference/pip_install/#editable-installs

My path hack doesn't help with alembic at all if I'm just trying to run commands like "heads", "merge", etc.

True, this only works with commands which can load the env.py, which includes alembic revision, as it turns out, but you're right about heads etc

sqlalchemy-bot commented 7 years ago

Changes by Michael Bayer (zzzeek):