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

Allow specifying script_location as resource specification #29

Closed
sqlalchemy-bot opened this Issue Feb 7, 2012 · 12 comments

Comments

Projects
None yet
1 participant
@sqlalchemy-bot

sqlalchemy-bot commented Feb 7, 2012

Migrated issue, originally created by Sok Ann Yap (@sayap)

To deploy the migration scripts more easily, I intend to bundle the script_location directory together with my .egg package. This requires alembic to support using resource specification for script_location, otherwise I have to keep changing the value in the .ini file whenever a newer .egg is deployed.

Attached is a diff for that. And thanks for alembic, it is awesome :)


Attachments: 29.1.patch | 29.patch | alembic-script-location.diff

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Feb 8, 2012

Michael Bayer (@zzzeek) wrote:

OK, can you try out the attached patch. This patch has you specify the name as "egg:" the way Python Paste (and others?) does, so that a windows filename like "C:/path/to/my/file" wouldn't be misinterpreted for example, and also keeps the pkg_resources import local to that usage to avoid an unnecessary dependency.

I've had several people ask me about this feature so thanks for showing me the idea !

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Feb 8, 2012

Changes by Michael Bayer (@zzzeek):

  • attached file 29.patch
@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Feb 8, 2012

Sok Ann Yap (@sayap) wrote:

The patch works after setting maxsplit to 1 for the first split. Perhaps it shall be set for the second split as well, just in case the file path contains a semicolon.

Anyway, I am not sure about the "egg:" prefix. Pyramid, for example, uses just ":" for its asset specification, and it only checks for os.path.isabs to handle windows filename. I suppose such checking is insufficient when the code runs on linux while the path is for windows, but that should be a rare (or invalid?) use case.

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Feb 8, 2012

Michael Bayer (@zzzeek) wrote:

Forgot the maxsplit, whoops. Does Pyramid distinguish between filesystem and pkg resources in the same way, just looking for a ":" ? A ":" seems like a pretty generic thing that might be in some weird quoted filesystem path. Looking at their docs at http://docs.pylonsproject.org/projects/pyramid/en/latest/narr/environment.html#examples, Pyramid seems to expect that all locations are already resource locations, so they wouldn't have this issue. More docs on this at http://docs.pylonsproject.org/projects/pyramid/en/latest/narr/assets.html#asset-specifications . That is, there's no guessing here, everything is a resource.

They support Paste so like here: http://docs.pylonsproject.org/projects/pyramid/en/latest/narr/environment.html#examples you see the "egg:" thing. Paste has you specify either "egg:" or "config:". In our case, this key is already a filesystem path in the default case so it seems like "egg:" as a prefix means we don't have to guess.

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Feb 8, 2012

Michael Bayer (@zzzeek) wrote:

we'll see what twitter says. http://twitter.com/zzzeek/status/167042538732142592

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Feb 8, 2012

Sok Ann Yap (@sayap) wrote:

This is the function in Pyramid that resolves asset specification by checking for semicolon and os.path.isabs:

https://github.com/Pylons/pyramid/blob/master/pyramid/asset.py#L11

I think PasteDeploy is a different beast, as it supports "egg:", "config:", "call:", and even just a string without semicolon to refer to another section. To add something similar to this issue to PasteDeploy, I actually modified the patch from http://groups.google.com/group/paste-users/browse_thread/thread/1f2ff6295426d1d5 to add "configspec:", which works the same way as the patch you attached here.

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Feb 8, 2012

Michael Bayer (@zzzeek) wrote:

okey doke here's using pyramid's idea as is, will commit once it works for you

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Feb 8, 2012

Changes by Michael Bayer (@zzzeek):

  • attached file 29.patch
@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Feb 8, 2012

Sok Ann Yap (@sayap) wrote:

Thanks. Patch works great for me :)

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Feb 8, 2012

Michael Bayer (@zzzeek) wrote:

[[https://bitbucket.org/zzzeek/alembic/changeset/72bb16079a554f6b6c0dd6b26c496d20d636c07a|72bb16079a554f6b6c0dd6b26c496d20d636c07a]] thanks !

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Feb 8, 2012

Changes by Michael Bayer (@zzzeek):

  • changed status to closed
@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented May 15, 2014

Michael Bayer (@zzzeek) wrote:

Issue #204 was marked as a duplicate of this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment