Add alembic test #102
Add alembic test #102
Conversation
jrcastro2
commented
Jun 6, 2023
•
jrcastro2
commented
- closes Groups membership support invenio-app-rdm#2186
| elif script.down_revision in revision_ids: | ||
| down_revision_index = revision_ids.index(script.down_revision) | ||
| revision_ids.insert(down_revision_index + 1, script.revision) | ||
| elif script.nextrev in revision_ids: |
There was a problem hiding this comment.
question: is this code reachable? The other two conditions state: if element in collection , elif element not in collection. That should cover all the possibilities?
There was a problem hiding this comment.
This covers all the possibilites except for the very first recipe of a branch, which has down_revision = None in that case we need to rely on nextrev.
There was a problem hiding this comment.
But then if script.down_revision = None, it would enter in the first statement I think
In [2]: a = []
In [3]: None in a
Out[3]: False
In [4]: None not in a
Out[4]: TrueThere was a problem hiding this comment.
follow up on branches and dependencies between branches:
what happens with a recipe that has down_revision = '1234' and depends_on = '5678'? It should run:5678 -> 1234 -> current.
| module_scripts.append(script) | ||
| revision_ids = _sort_revision_ids(module_scripts) | ||
| for revision_id in revision_ids: | ||
| ext.alembic.upgrade(target=revision_id) |
There was a problem hiding this comment.
question: what happens with branches? Two distinct alembic branches might have the same depends_on but no down_revision.
There was a problem hiding this comment.
Not sure I fully understand the question. We do get all the recipes of the branch we are willing to test, those recipes should always have down_revision (except for the fist recipe of the branch). When executing the upgrade for each recipe, it will execute the recipe and if it has any dependency (depends_on) it will execute it first.
There was a problem hiding this comment.
Ok then I think it's up to _sort_revision_ids to actually take into account these branches edge cases and depends_on.
jrcastro2
force-pushed
the
add-alembic-test
branch
from
8552fdf to
c4e602f
Compare
jrcastro2
force-pushed
the
add-alembic-test
branch
from
c4e602f to
ae37896
Compare
jrcastro2
force-pushed
the
add-alembic-test
branch
from
ae37896 to
0ddf0f5
Compare
|
This is not anymore needed |
