fix: fixed ecb ids drift between eba dpm releases - #264
fix: fixed ecb ids drift between eba dpm releases#264guillermo-garcia-1 wants to merge 1 commit into
Conversation
andres-sole
left a comment
There was a problem hiding this comment.
ECB org_id is still assigned with plain max+1, so it still drifts. The real 4.4 database has 3 organisations (1001, 1012, 1023), so ECB gets 1024, and it gets a different value as soon as an organisation is added upstream. That value is written to Operation.owner_id, and it is also part of the input to the concept GUID, so Concept.concept_guid and Operation.row_guid change between releases too.
To fix pin the ECB org_id, or remove org_id from the GUID input and use the fixed "ECB" acronym.
The base is also hardcoded while the id_prefix next to it is computed. DPM allocates IDs as IDPrefix * 10000000 + sequence: EBA has prefix 101 and its Operation IDs run from 1010000004 to 1010000897, and Release IDs follow the same rule (4.2.1 is 1010000003). The existing prefixes are 100, 101 and 102, so ECB gets 103, and 103 * 10000000 is exactly _ECB_ID_BASE. That only matches because of the max+1. If an organisation is added upstream and takes 103, ECB moves to 104 while the IDs still come from block 103, and that organisation writes rows inside the ECB block.
To fix pin the ECB id_prefix as well and derive the base from it, id_prefix * 10000000, so the prefix and the block cannot disagree.
Also the margin check only reads IDs below the floor, so it cannot detect native IDs above the floor. Test with a native operation_id of 1040000000, the check passes and the ECB operation gets 1040000001. The drift returns and nothing is reported. With the prefix problem above this case is realistic, not only theoretical. To fix fail or warn when rows at or above the floor were not written by ECB. For Operation, compare owner_id with the ECB org id. The other three tables need a join through operation_id.
The block itself is free today. I checked the 4.4 database and there are no rows at or above 1030000000 in Operation, OperationVersion, OperationNode or OperandReference, with about 20 million left to the native maximum.
OperationScope.operation_scope_id is not assigned here, and not in the scope calculator either, so the database assigns it as max+1 over the whole table and those IDs still drift. row_guid is stable because it comes from the operation_vid this PR fixes, so it may not matter. Is that intended, or should scopes go in the block too?
For follow up:
- ecb_validations_import.py:230 - "only ECB writes IDs >= floor" is the exact assumption that breaks if prefix 103 is taken upstream
- ecb_validations_import.py:241 - message hardcodes _ECB_ID_BASE but the function takes floor; tests pass 1000 and match on "_ECB_ID_BASE". Also: bumping the base is a planned drift, and Integer caps at 2147483647
- ecb_validations_import.py:205 - docstring overstates the guarantee; per-code IDs still shift with sorted(unique_codes) at :642
- ecb_validations_import.py:77 - databases imported with the old code end up with ECB IDs in two ranges; the PR description should state whether they need a fresh import. The reserved range should be documented in the module docstring
- ecb_validations_import.py:55 - error derives from Exception, not DpmCoreError (pre-existing)
- test file:649 - also assert the value equals _ECB_ID_BASE
- test file:314 - check is only tested through Organisation, never through the four real models
- test file:621 - 5000 rows inserted one at a time; that scale is not needed
- test file:630 - session not wrapped in try/finally, unlike the rest of the file
Summary
Checklist
ruff format,ruff check,mypy)pytest) with 100% branch coverage (coverage report --fail-under=100)Impact / Risk
Notes