feat(memstore): mem/db scriptable API + entity stringify (Benerator parity)#200
Merged
Conversation
…arity)
Unblocks memstore/memstore.ben.xml. Four independent gaps, one turned out to
already be closed.
1. .wgt.ent.csv (a normal headered CSV, test_id|flag|count, despite the
name) already reads correctly via <iterate> today - verified directly
rather than assumed (str.endswith(".csv") is a suffix match, no
more-specific branch intercepts it). Zero source change; added the
regression test only.
2. mem/db are now bound by id into the script namespace at registration
time (SetupContext.add_client for <database>/<mongodb>, MemstoreTask for
<memstore>) - not only inside <execute>. Both eval_namespace and
evaluate_python_expression's scope-building read from the same
underlying dict, so <execute>, <variable script=>, and <key script=> all
resolve mem/db identically regardless of statement order - verified with
a test that accesses mem BEFORE any <execute> has run, which a narrower
"only patch eval_namespace" fix would only pass by accident (via its
diff-writeback side effect, and only in the "after" ordering).
3. Memstore gained sumEntityColumn/entityCount/removeNotExistingIds
(Benerator's script-callable API). sumEntityColumn coerces via float()
since memstore rows sourced from CSV carry strings, not numbers - caught
by TDD (an empty-sum edge case: sum() defaults to int 0, not float, so
.is_integer() raised). removeNotExistingIds reuses
RdbmsClient.get_random_rows_by_columns unchanged, string-coercing both
sides of the id comparison since a DB-typed PK and a CSV-sourced memstore
id are typically different types for the same logical value.
4. A whole entity bound into a scalar column (<key script="person"> into a
varchar/Mongo field) now gets str(entity.to_dict()), not a crash.
Confirmed empirically before this fix: sqlite3.ProgrammingError /
bson.errors.InvalidDocument - so this only turns a crash into a correct
write, never changes behavior for what works today. BaseEntity defines
no __str__, so a bare str(value) would have written a non-deterministic
memory-address repr - one shared helper (stringify_if_entity) guards both
the RDBMS and Mongo write paths so they can't drift apart.
<evaluate assert="..."> (the 5th item in the original migration spec) needed
no CE change at all - it's pure Benerator syntax with no DATAMIMIC
equivalent; KeyVariableTask already passes a script's scalar result through
untouched. Purely a note for the separate converter tool, not code here.
Full suite 1452 passed/15 skipped, external_service_tests 95 passed/11
skipped, mypy/ruff clean.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Unblocks
memstore/memstore.ben.xml. Four independent gaps in the migration spec, one turned out to already be closed..wgt.ent.csvalready reads correctly via<iterate>today - verified directly rather than assumed (str.endswith(".csv")is a suffix match, no more-specific branch intercepts it). Zero source change; added the regression test only.mem/dbbound by id into the script namespace at registration time (SetupContext.add_clientfor<database>/<mongodb>,MemstoreTaskfor<memstore>) - not only inside<execute>. Verified with a test accessingmembefore any<execute>has run, which a narrower "only patcheval_namespace" fix would only pass by accident (via its diff-writeback side effect, and only in the "after" ordering).MemstoregainedsumEntityColumn/entityCount/removeNotExistingIds(Benerator's script-callable API).sumEntityColumncoerces viafloat()- caught a real edge case via TDD (sum()'s empty-case defaults to int0, not float, so.is_integer()raised).removeNotExistingIdsreusesRdbmsClient.get_random_rows_by_columnsunchanged, string-coercing both sides of the id comparison.<key script="person">into a varchar/Mongo field) now getsstr(entity.to_dict()), not a crash. Confirmed empirically before this fix:sqlite3.ProgrammingError/bson.errors.InvalidDocument- this only turns a crash into a correct write, never changes behavior for what works today. One shared helper (stringify_if_entity) guards both write paths so they can't drift apart.<evaluate assert="...">(the 5th item in the original migration spec) needed no CE change at all - pure Benerator syntax with no DATAMIMIC equivalent;KeyVariableTaskalready passes a script's scalar result through untouched. Just a note for the separate converter tool.Test plan
.wgt.ent.csvregression test.mem/dbaccessible via<key script>both before and after an<execute>call.sumEntityColumn/entityCount/removeNotExistingIdsunit-tested directly (no DSL entry point existed before item 2 landed).<execute>mem.sumEntityColumn(...)</execute>feeding a subsequentcount="{totalCount}".removeNotExistingIdsagainst a real sqlite table.to_dict()-based string after).