Repository navigation
Bump score_persistency to latest main (323950c) - #383
Merged
Merged
Conversation
Update score_persistency pin from 9ae529ba to 323950c4 and adapt the
integration to upstream changes:
- Drop obsolete local patches now provided upstream:
- 001-add-mod-need / 006-add-module-index-title: upstream ships
docs/module/index.rst with the mod__persistency need and a title.
- 002-rename-manual-verification-report-id: upstream's
module_verification_report.rst is now a :post_template:
module_verification_report report itself.
- 007-migrate-baselibs-rust-targets: upstream already uses
@score_baselibs//score/log_rust/score_log.
- Regenerate patches 003/004/005 (pip/imagefs/googletest non-dev) against
the new commit; generated cumulatively so they apply in sequence.
- Remove the reference_integration-owned verification-report wrapper and
point docs/verification_report/modules/index.rst at the mounted upstream
report, matching baselibs/logging.
- Regenerate known_good-derived Bazel fragments.
antonkri
requested review from
AlexanderLanin,
FScholPer,
MaximilianSoerenPollak,
PiotrKorkus,
dcalavrezo-qorix,
lurtz,
nradakovic,
opajonk and
pawelrutkaq
as code owners
October 2, 2026 07:07
…antics Upstream persistency (PR #417) removed the default-value fallback from get_value/get_value_as; they now only return written values and yield KeyNotFound for unwritten keys. Default values must be read explicitly via the new get_default_value API. Add a get_default_value_f64 helper (cpp method + rust fn) and route all scenario reads that expect a configured default through it: - default_values, reset_to_default, utf8_defaults: own-default reads - multi_instance_isolation: both the own-default reads and the cross-instance isolation checks now use get_default_value, which strengthens the default isolation assertion (absence => KeyNotFound instead of masked by get_value).
|
The created documentation from the pull request is available at: docu-html |
umaucher
approved these changes
Oct 2, 2026
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The overall-status tracker still references and reports data from the previous Persistency revision.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Bumps Persistency to the latest pinned revision and adapts integration tests, patches, and documentation to upstream changes.
Changes:
- Updates the Persistency commit and regenerates Bazel integration metadata.
- Removes obsolete patches and refreshes the remaining dependency patches.
- Migrates default-value scenarios and verification-report links to upstream APIs/docs.
| File | Description |
|---|---|
known_good.json |
Updates the Persistency pin and patch list. |
bazel_common/score_modules_target_sw.MODULE.bazel |
Regenerates the Persistency override. |
patches/persistency/001-add-mod-need.patch |
Removes an upstreamed docs patch. |
patches/persistency/002-rename-manual-verification-report-id.patch |
Removes an obsolete report patch. |
patches/persistency/003-pip-not-dev-dependency.patch |
Refreshes the pip dependency patch. |
patches/persistency/004-imagefs-not-dev-dependency.patch |
Refreshes the imagefs patch. |
patches/persistency/005-googletest-not-dev-dependency.patch |
Refreshes the googletest patch. |
patches/persistency/006-add-module-index-title.patch |
Removes an upstreamed title patch. |
patches/persistency/007-migrate-baselibs-rust-targets.patch |
Removes an upstreamed target migration. |
feature_integration_tests/test_scenarios/rust/src/internals/persistency/kvs_instance.rs |
Adds typed default-value retrieval. |
feature_integration_tests/test_scenarios/rust/src/scenarios/persistency/default_values.rs |
Uses the new default-value helper. |
feature_integration_tests/test_scenarios/rust/src/scenarios/persistency/multi_instance_isolation.rs |
Updates isolation checks. |
feature_integration_tests/test_scenarios/rust/src/scenarios/persistency/reset_to_default.rs |
Updates reset verification. |
feature_integration_tests/test_scenarios/rust/src/scenarios/persistency/utf8_defaults.rs |
Updates UTF-8 default reads. |
feature_integration_tests/test_scenarios/cpp/src/internals/persistency/kvs_instance.h |
Declares default-value retrieval. |
feature_integration_tests/test_scenarios/cpp/src/internals/persistency/kvs_instance.cpp |
Implements default-value retrieval. |
feature_integration_tests/test_scenarios/cpp/src/scenarios/persistency/default_values.cpp |
Updates default-value scenarios. |
feature_integration_tests/test_scenarios/cpp/src/scenarios/persistency/multi_instance_isolation.cpp |
Updates isolation checks. |
feature_integration_tests/test_scenarios/cpp/src/scenarios/persistency/reset_to_default.cpp |
Updates reset verification. |
feature_integration_tests/test_scenarios/cpp/src/scenarios/persistency/utf8_defaults.cpp |
Updates UTF-8 default reads. |
docs/verification_report/modules/persistency/persistency_verification_report.rst |
Removes the local report wrapper. |
docs/verification_report/modules/index.rst |
Links the upstream Persistency report. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| "rust_coverage_config": "ferrocene-coverage-per" | ||
| }, | ||
| "hash": "9ae529ba9f413976ff5c9948c6490afa51bbfdc3" | ||
| "hash": "323950c42431956ce76bf48c407a1709f114876d" |
anmittag
approved these changes
Oct 2, 2026
PiotrKorkus
approved these changes
Oct 5, 2026
antonkri
added a commit
to antonkri/reference_integration
that referenced
this pull request
Oct 6, 2026
The module integration test step (added in eclipse-score#385) is red on main because two of the three configured modules fail for module-internal reasons: - score_persistency: cit_rust fails to build its python test venv with "Collision resolution not yet implemented! Saw nodejs_wheel/executable.py twice" (surfaced by the eclipse-score#383 persistency bump to 323950c, which also dropped several bazel patches). - score_logging: the datarouter component tests fail with "TimeoutError: Datarouter not ready within 10.0s". Remove the integration_test_targets/integration_test_config for these two modules so the pipeline is green again, keeping score_lifecycle (which passes). The persistency/logging integration tests can be re-enabled once the underlying venv collision and datarouter timeout are fixed.
PiotrKorkus
pushed a commit
that referenced
this pull request
Oct 6, 2026
* Add psutil to feature integration test requirements The feature integration tests add a lifecycle persistency recovery test (tests/lifecycle/test_lifecycle_persistency_recovery.py) that imports psutil, but psutil was not declared in the test requirements. The fit_*_lifecycle_persistency targets therefore fail during pytest collection with "ModuleNotFoundError: No module named 'psutil'". Add psutil to requirements.txt and the generated lock so it is part of all_requirements and available to the feature integration test venv. * Drop failing persistency and logging module integration tests The module integration test step (added in #385) is red on main because two of the three configured modules fail for module-internal reasons: - score_persistency: cit_rust fails to build its python test venv with "Collision resolution not yet implemented! Saw nodejs_wheel/executable.py twice" (surfaced by the #383 persistency bump to 323950c, which also dropped several bazel patches). - score_logging: the datarouter component tests fail with "TimeoutError: Datarouter not ready within 10.0s". Remove the integration_test_targets/integration_test_config for these two modules so the pipeline is green again, keeping score_lifecycle (which passes). The persistency/logging integration tests can be re-enabled once the underlying venv collision and datarouter timeout are fixed.
antonkri
added a commit
that referenced
this pull request
Oct 6, 2026
The score_persistency KVS integration tests (//score/kvs/tests/test_cases:cit,
i.e. cit_cpp + cit_rust) could not build in reference_integration: building
their test venv failed with
× Collision resolution not yet implemented!
Saw nodejs_wheel/bin/corepack twice (then iniconfig, pytest, ...)
Root cause: the venv is assembled by score_tooling's score_virtualenv, which
does py_venv(deps = <@pip_tooling all_requirements> + reqs + ...). persistency
passes reqs = all_requirements of its own test hub (@pip_score_persistency_venv_test,
renamed by patch 003). That hub re-vendors score_tooling's
python_basics/requirements.txt (the test lock lists it as a compile src), so
basedpyright, iniconfig, nodejs-wheel-binaries, packaging, pluggy, pygments and
pytest exist in BOTH hubs. aspect_rules_py 1.5.2's venv tool cannot merge two
different repos that ship the same file (collision resolution is unimplemented
regardless of package_collisions=warning/ignore), so venv creation aborts.
This surfaced only after persistency was bumped to 323950c (#383) and the module
integration-test step was added (#385); it is the first time these tests ran in
reference_integration.
Fix (patches/persistency/006-dedupe-test-venv-pip-hub.patch): drop the seven
packages that score_tooling's @pip_tooling hub already provides from persistency's
test lock, so each package is contributed by exactly one hub. pytest and friends
come from @pip_tooling (added by score_virtualenv); persistency's hub keeps only
its own unique deps (psutil, pytest-env, pytest-html, pytest-metadata,
pytest-repeat, testing-utils, jinja2, markupsafe).
Verified locally (no docker): both
@score_persistency//score/kvs/tests/test_cases:python_tc_venv and
@score_persistency//score/kvs/tests/test_cases:cit build successfully with the
patch applied.
Re-enable the persistency integration_test_targets/integration_test_config in
known_good.json (removed in #386) so CI exercises the KVS integration tests.
antonkri
added a commit
that referenced
this pull request
Oct 7, 2026
…overrides (#388) * Enable persistency integration tests and fix test-venv pip hub collision The score_persistency KVS integration tests (//score/kvs/tests/test_cases:cit, i.e. cit_cpp + cit_rust) could not build in reference_integration: building their test venv failed with × Collision resolution not yet implemented! Saw nodejs_wheel/bin/corepack twice (then iniconfig, pytest, ...) Root cause: the venv is assembled by score_tooling's score_virtualenv, which does py_venv(deps = <@pip_tooling all_requirements> + reqs + ...). persistency passes reqs = all_requirements of its own test hub (@pip_score_persistency_venv_test, renamed by patch 003). That hub re-vendors score_tooling's python_basics/requirements.txt (the test lock lists it as a compile src), so basedpyright, iniconfig, nodejs-wheel-binaries, packaging, pluggy, pygments and pytest exist in BOTH hubs. aspect_rules_py 1.5.2's venv tool cannot merge two different repos that ship the same file (collision resolution is unimplemented regardless of package_collisions=warning/ignore), so venv creation aborts. This surfaced only after persistency was bumped to 323950c (#383) and the module integration-test step was added (#385); it is the first time these tests ran in reference_integration. Fix (patches/persistency/006-dedupe-test-venv-pip-hub.patch): drop the seven packages that score_tooling's @pip_tooling hub already provides from persistency's test lock, so each package is contributed by exactly one hub. pytest and friends come from @pip_tooling (added by score_virtualenv); persistency's hub keeps only its own unique deps (psutil, pytest-env, pytest-html, pytest-metadata, pytest-repeat, testing-utils, jinja2, markupsafe). Verified locally (no docker): both @score_persistency//score/kvs/tests/test_cases:python_tc_venv and @score_persistency//score/kvs/tests/test_cases:cit build successfully with the patch applied. Re-enable the persistency integration_test_targets/integration_test_config in known_good.json (removed in #386) so CI exercises the KVS integration tests. * Bump persistency to latest main and refresh integration-test overrides persistency fixed the test-venv pip hub collision upstream (#463 migrated to score_pytest from score_tools, #466 renamed the hub to pip_score_persistency_venv_test and dropped the unused python_tc_venv IDE venv). Bump the pinned commit to current main (961c0a0) so the reference_integration graph picks up those fixes directly. Consequences of the bump: - Drop 006-dedupe-test-venv-pip-hub.patch: the colliding python_tc_venv it worked around no longer exists upstream. - Regenerate 003/004/005 against the new MODULE.bazel (the old context no longer matched, so Bazel failed to apply them during fetch). - Add 006-tools / 007-sbom: the new main declares score_tools and score_sbom as dev_dependency, but reference_integration (root) needs them non-dev because the test BUILD loads @score_tools//score_pytest and the module root BUILD loads @score_sbom//:defs.bzl. Regenerated bazel_common/score_modules_target_sw.MODULE.bazel and refreshed MODULE.bazel.lock accordingly. * Pin score_tools to the $(locations) fix for persistency cit tests Persistency's integration test //score/kvs/tests/test_cases:cit failed in analysis: cit_cpp: label '...:cit_files' in $(location) expression expands to more than one file, please use $(locations ...) instead. score_pytest (from score_tools) only emits the correct $(locations ...) for multi-file srcs starting at a commit past the registry version 0.0.3. Persistency's own MODULE.bazel git_overrides score_tools to 4b48cec for exactly this fix, but Bzlmod ignores non-root overrides, so RI (root) was resolving score_tools 0.0.3 from the registry (still buggy). Add score_tools to known_good.json pinned to the same commit so the root module applies the override, regenerate the tooling MODULE fragment and refresh the lockfile. * Consolidate persistency dev_dependency flips into one patch The five patches 003-007 all edited the same file (MODULE.bazel) and four of them touched the same hunk region, so none of 005/006/007 applied standalone - they only worked as a sequential chain in exactly the known_good.json order, each carrying the previous ones' edits as context. Reordering or dropping any entry silently broke the rest. Replace them with a single patch containing two clean hunks. Same net effect: flip googletest, score_sbom, score_rules_imagefs, score_tools and the pip extension to non-dev, because their apparent names appear in BUILD/.bzl files that reference_integration evaluates (//:BUILD for the docs bundle, test_cases/BUILD for the pip hub). Extensions RI never evaluates, such as sbom_metadata and imagefs, stay dev on purpose. Verified: the patch applies standalone, and both @score_persistency//score/kvs/tests/test_cases:cit and @score_persistency//:docs_bundle resolve with --lockfile_mode=error.
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
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.
Updates the
score_persistencypin from9ae529bato323950c4(latestmain) and adapts the integration to upstream changes.Changes
001-add-mod-need/006-add-module-index-title: upstream now shipsdocs/module/index.rstwith themod__persistencyneed and a title.002-rename-manual-verification-report-id: upstream'smodule_verification_report.rstis itself a:post_template: module_verification_reportreport now.007-migrate-baselibs-rust-targets: upstream already uses@score_baselibs//score/log_rust/score_log.003/004/005(pip / imagefs / googletest marked non-dev) against the new commit; generated cumulatively so they apply in sequence.docs/verification_report/modules/index.rstat the mounted upstream report, matching the baselibs/logging pattern (avoids a duplicatedoc__persistency_verification_reportneed ID).known_good.json-derived Bazel fragments viascripts/known_good/update_module_from_known_good.py.Notes
MODULE.bazel.lockis unchanged: the bumped persistency deps are alldev_dependency, which Bazel ignores for non-root modules, so the resolved module graph does not change.bazel mod deps --lockfile_mode=updateconfirmed consistency and fetched persistency at the new commit, applying patches 003/004/005 cleanly.