MMO x DDL #186
MMO x DDL #186
Conversation
Package of operations that update 'visible columns' source definitions found in various annotations based on changes to named model elements.
Col, Key, FKey rename operations propogate name change to annotations.
| @@ -1,15 +1,16 @@ | |||
|
|
|||
| from __future__ import annotations | |||
|
|
|||
| import base64 | |||
There was a problem hiding this comment.
Hmmm... this must be what pycharm meant by "optimizing" imports on commit. I'm gonna not do that in the future. If this is annoying I can revert this part but its just a reordering as far as I see.
Col, Key, FKey drop operations propagate name change to annotations.
Table drop operation propagates name removal to annotations.
Schema drop operation propagates name removal to annotations.
Drop table fix to clean up its implicitly dropped fkeys by pruning their names from all other table's annotations.
deriva/core/ermrest_model.py
Outdated
| if update_mappings: | ||
| for fkey in self.foreign_keys: | ||
| mmo.prune(self.schema.model, [fkey.constraint_schema.name, fkey.constraint_name]) | ||
| self.schema.apply() |
There was a problem hiding this comment.
Doesn't this have to be self.schema.model.apply? Changes could be global across any schema in the model, right?
The absolute imports work better when invoking whole/sub trees of tests.
Fixing the apply() calls to always be rooted on the model not the current object's parent, since the annotation changes apply potentially throughout the entire catalog model.
Added a 'deferred' mode and made the 'update_mappings' parameter based on an enum representing the three modes. Refactored the pre/post cond logic in the test_mmo_drop unittests as it seemed over-engineered.
MMO primitives to find, prune, replace constraint names based on the schema name part only.
Support for updating mappings after a Table.alter(schema_name=...) change.
Fixed a bug in the bare constraint name case for schema name changes to be reflected in vizcol and vizfkey annotations.
On `schema.alter(schema_name=...)` all constraint names throughout supported annotation structures are updated.
| nochange = NoChange() | ||
|
|
||
| class UpdateMappings (str, Enum): | ||
| """Update Mappings flag enum""" | ||
| no_update = '' | ||
| deferred = 'deferred' | ||
| immediate = 'immediate' | ||
|
|
There was a problem hiding this comment.
@karlcz think this is okay this way? or would it be better to have variable instances of the enum of say no_update = UpdateMappings.no_update analogous to your nochange = NoChange() instance?
There was a problem hiding this comment.
I think this is fine. You can use it as UpdateMappings.no_update etc.
There was a problem hiding this comment.
However, I'm not sure I like the empty string... I think I'd also spell it out.
There was a problem hiding this comment.
Nevermind, I see you did it to make the "falsish" test easier to write below...
|
I think this is good to merge now. I didn't actually try to run the test suite though, as I'm not sure if/how it depends on the other open PR for mmo...? |
|
Cool. I have run the tests. It all succeeds.
|
Integration of MMO with DDL operations.