Introducing schema revisions #176
Conversation
|
@jacquerie @michamos @david-caro this is an initial proposal (still implementing) for supporting ledgers and revisions. Note this is super WIP |
kaplun
force-pushed
the
revisions
branch
3 times, most recently
from
f1f7b75 to
6105f28
Compare
Signed-off-by: Samuele Kaplun <[email protected]>
|
new element added in #173 |
|
|
||
| def upgrade(json): | ||
| schema, current_revision = get_schema_and_revision(json) | ||
| for revision, rule in LEDGERS['schema']: |
There was a problem hiding this comment.
Here 'schema' should not be a string but the schema var right?
There was a problem hiding this comment.
This only works if the versions in the ledger are ordered from smallest to largest. This should probably be tested and either documented or sorted in the right order in this loop.
| return latest_revisions | ||
|
|
||
|
|
||
| LATEST_SCHEMA_REVISIONS = build_latest_schema_revisions() |
There was a problem hiding this comment.
why do you need a global variable with the info? Can't you just call the function when you need it?
I'm a bit troubled by executing code right on import...
There was a problem hiding this comment.
Memoizing might be nicer ;)
| return json | ||
|
|
||
|
|
||
| def first_revision_rule(json): |
There was a problem hiding this comment.
Should we name them in as strict way? something like upgrade_from_base_to_0.0.1, that might be easier to read/scale than 'first/second/third...'.
There was a problem hiding this comment.
alternatively, we could use descriptive names like: new_foo_field.
| assert latest_schema_revisions['journals'] == '0.0.1' | ||
|
|
||
|
|
||
| def test_get_schema_and_revision(): |
There was a problem hiding this comment.
This looks like a good candidate to use pytest.mark.parametrize thingie.
There was a problem hiding this comment.
you could also simplify to
result = utils.get_schema_and_revision('literature-1.2.3.json')
expected = 'literature', '1.2.3'
assert result == expectedinstead of unpacking the tuples.
| LEDGERS = build_ledger() | ||
|
|
||
|
|
||
| def upgrade(json): |
There was a problem hiding this comment.
what is 'json'? calling rule(json) suggests it is a record, but get_schema_and_revision(json) suggests that it is a string instead.
|
|
||
| def upgrade(json): | ||
| schema, current_revision = get_schema_and_revision(json) | ||
| for revision, rule in LEDGERS['schema']: |
There was a problem hiding this comment.
This only works if the versions in the ledger are ordered from smallest to largest. This should probably be tested and either documented or sorted in the right order in this loop.
| return json | ||
|
|
||
|
|
||
| def first_revision_rule(json): |
There was a problem hiding this comment.
alternatively, we could use descriptive names like: new_foo_field.
| assert latest_schema_revisions['journals'] == '0.0.1' | ||
|
|
||
|
|
||
| def test_get_schema_and_revision(): |
There was a problem hiding this comment.
you could also simplify to
result = utils.get_schema_and_revision('literature-1.2.3.json')
expected = 'literature', '1.2.3'
assert result == expectedinstead of unpacking the tuples.
|
BTW, is semver really needed here? if the change is not incompatible, the records don't need to be upgraded. |
|
@david-caro I think this branch can be considered dead? Has it been decided how to move on with versioning? |
|
Well, we decided long ago to go with this, but nothing has been done so far (that I know of) |
|
I think @jmartinm wanted to investigate whether we could use alembic instead for that, but don't know if he ever did. |
|
We can close this. |
Closes #9, Closes #10
Signed-off-by: Samuele Kaplun [email protected]