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.

Sign up for GitHub

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

Jump to bottom

Add lock on object creation #5194

Merged
merged 2 commits into from
Dec 13, 2024
Merged

Add lock on object creation #5194

merged 2 commits into from
Dec 13, 2024
+312 −46

Conversation

LucasG0
Copy link
Contributor

@LucasG0 LucasG0 commented Dec 11, 2024 •
edited
Loading

Fixes #4523

This PR adds locks on main branch on:

  • Object kind for IPPrefix.mutate_create / mutate_update / mutate_delete
  • Object kind for IPAddress.mutate_create / mutate_update (mutate_delete has a simpler implementation than IPPrefix one)
  • Object and/or its generics kinds for InfrahubMutationMixin.mutate_create / mutate_update

Also moves tests/integration/ipam tests to tests/functional/ipam

@github-actions github-actions bot added the group/backend Issue related to the backend (API Server, Git Agent) label Dec 11, 2024
@codspeed-hq CodSpeed HQ
Copy link

codspeed-hq bot commented Dec 11, 2024 •
edited
Loading

CodSpeed Performance Report

Merging #5194 will not alter performance

Comparing lgu-add-lock (3e3f187) with release-1.1 (f76fa3c)

Summary

✅ 10 untouched benchmarks

@LucasG0 LucasG0 force-pushed the lgu-add-lock branch 4 times, most recently from 0a24ff8 to b81d63b Compare December 12, 2024 18:49
@LucasG0 LucasG0 requested a review from dgarros December 12, 2024 18:51
@LucasG0 LucasG0 force-pushed the lgu-add-lock branch 2 times, most recently from f1d41ae to 5a35b96 Compare December 12, 2024 19:18
@LucasG0 LucasG0 requested a review from a team December 12, 2024 19:20
dgarros
dgarros reviewed Dec 13, 2024
View reviewed changes
Copy link
Collaborator

@dgarros dgarros left a comment

Choose a reason for hiding this comment

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

Looks good
One important point missing is that right now the lock(s) will be applied on all branches not just the default branch

backend/infrahub/lock.py Outdated Show resolved Hide resolved
backend/infrahub/lock.py Outdated Show resolved Hide resolved
backend/tests/integration/ipam/test_load_concurrent_prefixes.py Outdated Show resolved Hide resolved
backend/infrahub/graphql/mutations/main.py Outdated Show resolved Hide resolved
backend/infrahub/graphql/mutations/ipam.py Outdated Show resolved Hide resolved
@LucasG0 LucasG0 force-pushed the lgu-add-lock branch 6 times, most recently from a76064b to de8731b Compare December 13, 2024 11:48
@LucasG0 LucasG0 requested a review from dgarros December 13, 2024 11:48
dgarros
dgarros approved these changes Dec 13, 2024
View reviewed changes
backend/infrahub/graphql/mutations/ipam.py Outdated Show resolved Hide resolved
backend/infrahub/graphql/mutations/ipam.py Outdated Show resolved Hide resolved
backend/infrahub/graphql/mutations/ipam.py Outdated Show resolved Hide resolved
backend/infrahub/graphql/mutations/main.py Outdated Show resolved Hide resolved
backend/infrahub/graphql/mutations/main.py Outdated Show resolved Hide resolved
@LucasG0 LucasG0 merged commit 938522d into release-1.1 Dec 13, 2024
30 checks passed
@LucasG0 LucasG0 deleted the lgu-add-lock branch December 13, 2024 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dgarros dgarros dgarros approved these changes

Assignees
No one assigned
Labels
group/backend Issue related to the backend (API Server, Git Agent)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@LucasG0 @dgarros