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

Creation of thing's policy is atomic with creation the of thing itself #1581

Merged

Conversation

alstanchev
Copy link
Contributor

This PR adds atomic behaviour to the creation of thing and its policy.
In the rare case of failing to create a thing, its policy will be rolled back (deleted) in order to not fail create retry.

@alstanchev
Copy link
Contributor Author

@thjaeckle can you take a look when you can please.

@alstanchev alstanchev requested a review from thjaeckle February 21, 2023 10:21
Signed-off-by: Stanchev Aleksandar <aleksandar.stanchev@bosch.io>

Signed-off-by: Stanchev Aleksandar <aleksandar.stanchev@bosch.io>
@alstanchev alstanchev force-pushed the feature/atomic-thing-create branch from 51a0043 to 77b1c03 Compare February 22, 2023 08:42
@thjaeckle
Copy link
Member

thjaeckle commented Feb 22, 2023

@alstanchev I will take a look soon.
Do you work on a unit test? That can probably only be tested with mocking and producing the "thing creation failure".

Copy link
Member

@thjaeckle thjaeckle left a comment

Choose a reason for hiding this comment

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

I added a first round of review comments.

Signed-off-by: Stanchev Aleksandar <aleksandar.stanchev@bosch.io>
@alstanchev alstanchev force-pushed the feature/atomic-thing-create branch from 01737fb to 594ac98 Compare March 7, 2023 11:43
@alstanchev alstanchev requested a review from thjaeckle March 7, 2023 11:44
@alstanchev alstanchev marked this pull request as ready for review March 7, 2023 11:45
@thjaeckle thjaeckle added this to the 3.2.0 milestone Mar 7, 2023
Copy link
Member

@thjaeckle thjaeckle left a comment

Choose a reason for hiding this comment

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

Hey @alstanchev
That looks great - I had a look at the changes + the tests and are confident that improves resiliency a lot.
Good stuff 👍

@thjaeckle thjaeckle merged commit 6269376 into eclipse-ditto:master Mar 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants