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

feat(ParticipantStore): add SQL store #43

Merged

Conversation

wolf4ood
Copy link
Contributor

What this PR changes/adds

Add SQL store implementation for ParticipantStore

Why it does that

Durability of participants and scalability for the Registration service

Linked Issue(s)

Closes #41

Checklist

  • added appropriate tests?
  • performed checkstyle check locally?
  • added/updated copyright headers?
  • documented public classes/methods?
  • added/updated relevant documentation?
  • added relevant details to the changelog? (skip with label no-changelog)
  • formatted title correctly? (take a look at the CONTRIBUTING and styleguide for details)

@wolf4ood wolf4ood added the enhancement New feature or request label Oct 11, 2022
@wolf4ood wolf4ood force-pushed the feature/41_sql_participant_store branch 4 times, most recently from 5da0d01 to e88b138 Compare October 12, 2022 09:06
@wolf4ood wolf4ood marked this pull request as ready for review October 12, 2022 09:13
@wolf4ood wolf4ood force-pushed the feature/41_sql_participant_store branch from e88b138 to 979994a Compare October 12, 2022 09:16
testFixturesImplementation("org.junit.jupiter:junit-jupiter-params:${jupiterVersion}")
testFixturesRuntimeOnly("org.junit.jupiter:junit-jupiter-engine:${jupiterVersion}")
testFixturesImplementation("org.assertj:assertj-core:${assertj}")

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am a bit concerned about pulling dependencies on other extensions and on test libraries into the *-spi directory. But maybe it's acceptable if it's only test fixture implementations... @paullatzelsperger what do you think?

Copy link
Contributor Author

@wolf4ood wolf4ood Oct 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bscholtes1A the deps to authority-spi can be easily removed since it's only one fixture. But since the model is there it made sense to use that module fixtures

The test engine it's needed. We did the same thing on the other stores by providing a base test for all the store implementation like here
https://github.com/eclipse-dataspaceconnector/DataSpaceConnector/blob/main/spi/control-plane/contract-spi/build.gradle.kts#L31

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bscholtes1A @wolf4ood I would slightly lean toward ben's position, i.e. removing the dependency onto dataspace-authority-spi, because we're only using a simple factory method, but I don't think this is a big issue either way.

@wolf4ood wolf4ood force-pushed the feature/41_sql_participant_store branch from 979994a to 48794bf Compare October 12, 2022 12:46
@wolf4ood wolf4ood force-pushed the feature/41_sql_participant_store branch from 48794bf to 1bee240 Compare October 12, 2022 14:02
@wolf4ood wolf4ood force-pushed the feature/41_sql_participant_store branch from 1bee240 to 20e07b6 Compare October 12, 2022 15:56
@paullatzelsperger paullatzelsperger merged commit 709b950 into eclipse-edc:main Oct 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Persistent ParticipantStore - PostgreSQL extension
3 participants