Skip to content

Conversation

@DavSanchez
Copy link
Contributor

@DavSanchez DavSanchez commented Oct 28, 2025

What this PR does / why we need it

Introduces a new trait OpAMPDataStore and makes K8sStore and a new FileStore implement it for operations regarding OpAMP data (CRUD remote config, identifiers, read local config, etc).

This way many of the structures that were originally dedicated for each implementation (Storer, ConfigRepositoryXXX) and contained mostly identical behavior have been replaced with a unique implementation that is generic over the new trait.

I tried to remove the intermediate traits (InstanceIDStorer, InstanceIDGetter, ConfigRepository) so the cleanup was even more deep and was a net removal of code, but I encountered some unexpected issues, mainly that OpAMPDataStore is not easy to mock with mockall due to it being highly generic, and also the fact that I would need to touch even more unrelated code to do that cleanup. So I leave this for a subsequent PR that might probably involve manual implementations of OpAMPDataStore mocks. This PR could be merged separately from that change.

Special notes for your reviewer

The commits called replace XXXX with generic one and remove redundant XXX impls do the replacement and cleanup respectively, so they touch quite a bit of code (for example, many tests for the on host implementations were moved from the storer/file modules and placed in file_store instead, there might be redundant tests there!).

Just a heads up in case you want to review them separately!

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

  • Provided a meaningful title following conventional commit style.
  • Included a detailed description for the Pull Request.
  • Documentation under docs is aligned with the change.
  • Follows guidelines for Pull Requests in CONTRIBUTING.md.
  • Follows log level guidelines.

@vjripoll vjripoll force-pushed the restructure-on-host-folders branch 2 times, most recently from b485354 to d6992c4 Compare October 28, 2025 09:17
@DavSanchez DavSanchez force-pushed the refactor/unified-host-fs-store branch from 9826e99 to 22dca1e Compare October 28, 2025 09:45
@vjripoll vjripoll force-pushed the restructure-on-host-folders branch from d6992c4 to f337555 Compare October 28, 2025 10:50
@DavSanchez DavSanchez force-pushed the refactor/unified-host-fs-store branch from 22dca1e to dcced0b Compare October 28, 2025 11:09
@vjripoll vjripoll force-pushed the restructure-on-host-folders branch from f337555 to 5f60349 Compare October 28, 2025 11:10
@DavSanchez DavSanchez force-pushed the refactor/unified-host-fs-store branch from dcced0b to 869c899 Compare October 28, 2025 11:12
@DavSanchez DavSanchez added onhost k8s-extended-e2e Trigger extended k8s e2e on a PR onhost-extended-e2e Execution of on host e2e in the current branch labels Oct 28, 2025
@vjripoll vjripoll force-pushed the restructure-on-host-folders branch from 5f60349 to 26475f9 Compare October 28, 2025 11:37
@DavSanchez DavSanchez force-pushed the refactor/unified-host-fs-store branch 3 times, most recently from 80905d0 to 6c96cd7 Compare October 28, 2025 12:54
@vjripoll vjripoll force-pushed the restructure-on-host-folders branch from 26475f9 to 19c3502 Compare October 28, 2025 14:38
@DavSanchez DavSanchez force-pushed the refactor/unified-host-fs-store branch 5 times, most recently from f1d3708 to 2f0c676 Compare October 29, 2025 01:39
@DavSanchez DavSanchez marked this pull request as ready for review October 29, 2025 02:08
@DavSanchez DavSanchez requested a review from a team as a code owner October 29, 2025 02:08
@vjripoll vjripoll force-pushed the restructure-on-host-folders branch 3 times, most recently from d7395b0 to 206ec88 Compare October 29, 2025 14:46
@DavSanchez DavSanchez force-pushed the refactor/unified-host-fs-store branch from 7922857 to 41c8c13 Compare October 29, 2025 15:28
@DavSanchez DavSanchez force-pushed the refactor/unified-host-fs-store branch from bd490c5 to 2f3cd78 Compare November 6, 2025 17:10
@DavSanchez DavSanchez enabled auto-merge (squash) November 6, 2025 17:14
@DavSanchez DavSanchez merged commit 79e2e30 into main Nov 6, 2025
37 of 38 checks passed
@DavSanchez DavSanchez deleted the refactor/unified-host-fs-store branch November 6, 2025 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

k8s-extended-e2e Trigger extended k8s e2e on a PR onhost onhost-extended-e2e Execution of on host e2e in the current branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants