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

requies minimum number of characters to match version numbers in commands #534

Closed
claweyenuk opened this issue Feb 13, 2019 · 6 comments
Closed

Comments

@claweyenuk
Copy link

@claweyenuk claweyenuk commented Feb 13, 2019

Can version matching get more strict?

https://github.com/sqlalchemy/alembic/blob/master/alembic/script/revision.py#L376

Ideally, something like if len(resolved_id) < 3 set revs = [].

I went to downgrade an database, and accidentally typed alembic downgrade 1 instead of alembic downgrade -1 and happened to have a single revision start with a "1", so the downgrade continued.

DB migrations can result in data loss, I think it's ok to require a certain number of characters when identifying the migration.

@zzzeek
Copy link
Member

@zzzeek zzzeek commented Feb 13, 2019

this is acceptable if someone wants to work on it.

@zzzeek zzzeek changed the title Make version matching more strict requies minimum number of characters to match version numbers in commands Feb 13, 2019
@claweyenuk
Copy link
Author

@claweyenuk claweyenuk commented Feb 13, 2019

Great! Thanks!

I'll plan to do it in the next few weeks (though if anyone sees this message, feel free to pick this change up)

@zzzeek
Copy link
Member

@zzzeek zzzeek commented Feb 13, 2019

please ensure your PR has tests in tests/test_command.py

@claweyenuk
Copy link
Author

@claweyenuk claweyenuk commented Feb 13, 2019

Will do

@sqla-tester
Copy link
Collaborator

@sqla-tester sqla-tester commented Sep 17, 2019

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

Only allow partial revision match for > 3 characters https://gerrit.sqlalchemy.org/1466

@claweyenuk
Copy link
Author

@claweyenuk claweyenuk commented Sep 17, 2019

Thanks for making a fix. (Sorry I never got around to it). This is exactly what I was thinking of.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants