Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

delete checkpointer abstraction #2054

Merged
merged 1 commit into from
Dec 12, 2024
Merged

delete checkpointer abstraction #2054

merged 1 commit into from
Dec 12, 2024

Conversation

edmundnoble
Copy link
Contributor

Replace the checkpointer abstraction with a concrete CheckpointerResources type. Consider the original RelationalCheckpointer code to be the "internal" portion of the checkpointer. Use qualified imports to refer to the checkpointer from PactService so that it's clear what's owned by it.

@edmundnoble edmundnoble requested a review from chessai December 4, 2024 04:16
@larskuhtz
Copy link
Contributor

I think the term CheckpointerResources is misleading. In the Chainweb code base the suffix Resources is usually used for ephemeral records that exist only during initialization (and are supposed to be garbage collected once the services are started). The purpose of those records is it to initialize and distribute shared backend components to top-level components in Chainweb node before starting node services.

@edmundnoble
Copy link
Contributor Author

Yeah fair point, I can rename it to Checkpointer.

@edmundnoble edmundnoble requested a review from larskuhtz December 4, 2024 17:43
@larskuhtz
Copy link
Contributor

Yeah fair point, I can rename it to Checkpointer.

Alternatively, we could also move away from the current (not ideal) terminology and use a more specific term for those ephemeral records. It's just to avoid confusion.

@edmundnoble edmundnoble force-pushed the push-qzuvyrzlpvwn branch 2 times, most recently from 4370147 to d891e23 Compare December 4, 2024 18:34
Replace the checkpointer abstraction with a concrete
`CheckpointerResources` type. Consider the original
RelationalCheckpointer code to be the "internal" portion of the
checkpointer. Use qualified imports to refer to the checkpointer from
PactService so that it's clear what's owned by it.
@edmundnoble edmundnoble merged commit 00b90c6 into pact5 Dec 12, 2024
3 of 10 checks passed
@edmundnoble edmundnoble deleted the push-qzuvyrzlpvwn branch December 12, 2024 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants