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

Database migrations: improvements and fixes #3570

Merged
merged 2 commits into from Feb 7, 2017

Conversation

Projects
None yet
2 participants
@nsoranzo
Copy link
Member

commented Feb 7, 2017

  • Consolidate exception logging
  • Align engine_true() and engine_false() methods
  • Remove unused now variables
  • Fix a bunch of database migration bugs

nsoranzo added some commits Feb 7, 2017

Consolidate exception logging in db migrations
Also:
- align engine_true() and engine_false() methods
- remove unused 'now' variables
@dannon

This comment has been minimized.

Copy link
Member

commented Feb 7, 2017

@nsoranzo Instead of engine_true() / engine_false(), is there a reason we can't use http://docs.sqlalchemy.org/en/latest/core/sqlelement.html?highlight=true#sqlalchemy.sql.expression.true ?

We use this elsewhere in Galaxy when we need to resolve to a true/false type.

(and, can you elaborate on the 'fix a bunch of database migration bugs'? Makes me worry a bit, I'd like to know what else was broken! :))

@dannon

This comment has been minimized.

Copy link
Member

commented Feb 7, 2017

(we talked through some of this on IRC, swapping to sqlalchemy.true() might be another nice improvement down the road, but definitely isn't a requirement here)

@dannon dannon merged commit 68752b7 into galaxyproject:dev Feb 7, 2017

5 checks passed

api test Build finished. 261 tests run, 0 skipped, 0 failed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
framework test Build finished. 138 tests run, 0 skipped, 0 failed.
Details
integration test Build finished. 24 tests run, 0 skipped, 0 failed.
Details
toolshed test Build finished. 580 tests run, 0 skipped, 0 failed.
Details

@nsoranzo nsoranzo deleted the nsoranzo:database_migrations branch Feb 7, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.