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

store: Add transactions #48

Merged
merged 1 commit into from
Nov 6, 2024
Merged

store: Add transactions #48

merged 1 commit into from
Nov 6, 2024

Conversation

tupyy
Copy link
Collaborator

@tupyy tupyy commented Oct 25, 2024

Transactions are needed because we must leave the db clean if something happens while mutating data. Currently, we have only one resource source but, with the new multi source model, we'll need to mutate two resources agent and source in the same operation. This requires that the op is done in a transaction.

This commit adds transaction support into the store. The code is largly based on the transaction implementation found in other RH service (e.g. uhc-service-account) but is has been made simplier to fit our needs.

The transactions are wrapped into a context and passed to the store when services are asking for source repository (for now, we have only one. Soon to be more). The store checks the context and if it finds a transaction it will use it to create the source repository. Otherwise, it uses its own db connection.

Methods for commit and rollback are added so a service can rollback or commit a transaction if an error occurs.

Signed-off-by: Cosmin Tupangiu [email protected]

@bardielle
Copy link
Collaborator

/retest

@machacekondra
Copy link
Collaborator

Looks good. I think adding the deploy-db dependency for test Makefile target should resolve the issue. Or do we want to provision the DB in a different way?

@tupyy
Copy link
Collaborator Author

tupyy commented Oct 30, 2024

Looks good. I think adding the deploy-db dependency for test Makefile target should resolve the issue. Or do we want to provision the DB in a different way?

either that or adding a service in the github actions. But we need to migrate the db and for that I was thinking adding a new command to planner-api migrate.

Transaction are needed because we have to leave the db in a clean state
if something happen while mutating data. Currently, we have only one
resource _source_ but, with the new multi source model, we'll need to
mutate two resources _agent_ and _source_ in the same operation.
This requires that the op is done in a transaction.

This commit adds transaction support into the store. The code is largly
based on the transaction implementation found in other RH service
(e.g. uhc-service-account) but is has been made simplier to fit our
needs.

The transactions are wrapped into a context and passed to store when
services are asking for _source_ repository (for now, we have only one.
Soon to be more). The store checks the context and if it finds a
transaction it will use it to create the source repository. Otherwise,
it uses its own db connection.

Methods for _commit_ and _rollback_ are added so a service can rollback
or commit a transaction if an error occurs.

Signed-off-by: Cosmin Tupangiu <[email protected]>
ctx, err := store.NewTransactionContext(context.TODO())
Expect(err).To(BeNil())

source, err := store.Source().Create(ctx, api.SourceCreate{Name: "test", SshKey: "some key"})
Copy link
Collaborator

Choose a reason for hiding this comment

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

I really need to add the validation to SSH key ;)

@machacekondra machacekondra merged commit 289dc36 into kubev2v:main Nov 6, 2024
9 checks passed
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