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

cli: alembic_version table manual drop #74

Merged
merged 1 commit into from
May 19, 2017

Conversation

dinosk
Copy link
Member

@dinosk dinosk commented Apr 13, 2017

  • Fixes sequence bug in transaction table.

  • Adds to the cli drop command the alembic_version table.
    Addresses #72.

Signed-off-by: Dinos Kousidis konstantinos.kousidis@cern.ch

)
op.execute(CreateSequence(Sequence("transaction_id_seq")))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will work only for Postgres - can we investigate an option with autoincrement?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jirikuncar right, I added autoincrement=True and flask db drop works fine after the alembic commands, but there is no sequence created:

+-------------+-----------------------------+-------------+
| Column      | Type                        | Modifiers   |
|-------------+-----------------------------+-------------|
| issued_at   | timestamp without time zone |             |
| id          | bigint                      |  not null   |
| remote_addr | character varying(50)       |             |
+-------------+-----------------------------+-------------+
Indexes:
    "pk_transaction" PRIMARY KEY, btree (id)

Time: 0.015s
postgres@localhost:invenio> \ds
+----------+--------+--------+---------+
| Schema   | Name   | Type   | Owner   |
|----------+--------+--------+---------|
+----------+--------+--------+---------+

so it differs slightly from flask db create.

@dinosk dinosk changed the title WIP alembic: fix of bug in transaction table with sequences cli: alembic_version table manual drop Apr 28, 2017
@dinosk dinosk force-pushed the alembic-fix branch 4 times, most recently from 2fd147f to fb87a27 Compare May 5, 2017 12:51
@@ -69,3 +70,12 @@ def rebuild_encrypted_properties(old_key, model, properties):
model.query.filter_by(**primary_key_fields).\
update(update_values)
db.session.commit()


def create_alembic_version_table():
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I add this so that we can use alembic (upgrade|downgrade) after creating the db with db create

@@ -28,7 +28,7 @@
from flask import current_app
from sqlalchemy.engine import reflection

from invenio_db import db
import invenio_db
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This had to be changed because it was causing a circural import with the from .utils import create_alembic_verson_table in cli.py

table.drop(bind=_db.engine, checkfirst=True)
try:
table.drop(bind=_db.engine, checkfirst=True)
except InternalError:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to add the try except because table.drop(bind=_db.engine, checkfirst=True) doesn't work after alembic upgrade heads and schema_dropper.connection.execute(DropTable(table)) doesn't work after db create

@@ -28,7 +28,7 @@
from flask import current_app
from sqlalchemy.engine import reflection

from invenio_db import db
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we change it to from .shared import db?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah yes

Copy link
Member

@nharraud nharraud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, I'll just add some tests.

@nharraud nharraud force-pushed the alembic-fix branch 3 times, most recently from 9bf1140 to 9083067 Compare May 19, 2017 08:42
- Adds to the cli drop command the alembic_version table, as it
  is not registered in db.metadata.sorted_tables and remains in the
  db after db drop. (closes inveniosoftware#72)

- Handles drop sequence bug in transaction table.
  (closes inveniosoftware/invenio-accounts#195)

- Adds a function to create the alembic_version table in the utils.py,
  to sync `db create` and `alembic ugprade heads`.

Signed-off-by: Dinos Kousidis <konstantinos.kousidis@cern.ch>
@nharraud nharraud merged commit 9083067 into inveniosoftware:master May 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants