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

[management] Refactor peers to use store methods #2893

Open
wants to merge 86 commits into
base: main
Choose a base branch
from

Conversation

bcmmbaga
Copy link
Contributor

@bcmmbaga bcmmbaga commented Nov 14, 2024

Describe your changes

Remove calls to get and save whole account in peers operations.

Issue ticket number and link

Checklist

  • Is it a bug fix
  • Is a typo/documentation fix
  • Is a feature enhancement
  • It is a refactor
  • Created tests that fail without the change (if possible)
  • Extended the README / documentation, if necessary

Signed-off-by: bcmmbaga <[email protected]>
…unt-refactoring

# Conflicts:
#	management/server/sql_store.go
…nt-refactoring

# Conflicts:
#	management/server/sql_store.go
Signed-off-by: bcmmbaga <[email protected]>
Signed-off-by: bcmmbaga <[email protected]>
Signed-off-by: bcmmbaga <[email protected]>
# Conflicts:
#	management/server/group.go
#	management/server/group/group.go
#	management/server/setupkey.go
#	management/server/sql_store.go
#	management/server/status/error.go
#	management/server/store.go
Signed-off-by: bcmmbaga <[email protected]>
Signed-off-by: bcmmbaga <[email protected]>
Signed-off-by: bcmmbaga <[email protected]>
Signed-off-by: bcmmbaga <[email protected]>
@bcmmbaga bcmmbaga force-pushed the peers-get-account-refactoring branch 2 times, most recently from e8a5d16 to 9950683 Compare November 18, 2024 20:35
Signed-off-by: bcmmbaga <[email protected]>
@bcmmbaga bcmmbaga force-pushed the peers-get-account-refactoring branch from 9950683 to 20fc8e8 Compare November 18, 2024 21:54
@bcmmbaga bcmmbaga marked this pull request as ready for review November 19, 2024 07:09
Signed-off-by: bcmmbaga <[email protected]>
@bcmmbaga bcmmbaga force-pushed the peers-get-account-refactoring branch from 21f57c0 to 4abf9ad Compare November 21, 2024 13:49
Signed-off-by: bcmmbaga <[email protected]>
@bcmmbaga bcmmbaga force-pushed the peers-get-account-refactoring branch from 4abf9ad to 82746d9 Compare November 21, 2024 14:15
# Conflicts:
#	management/server/policy.go
#	management/server/posture_checks_test.go
#	management/server/sql_store.go
#	management/server/sql_store_test.go
#	management/server/status/error.go
#	management/server/store.go
#	management/server/testdata/extended-store.sql
# Conflicts:
#	management/server/sql_store_test.go
#	management/server/status/error.go
#	management/server/testdata/extended-store.sql
Base automatically changed from nameserver-get-account-refactoring to main November 26, 2024 16:39
…unt-refactoring

# Conflicts:
#	management/server/account.go
#	management/server/http/peers_handler.go
#	management/server/peer.go
# Conflicts:
#	management/server/sql_store_test.go
… user peers with empty userID

Signed-off-by: bcmmbaga <[email protected]>
Signed-off-by: bcmmbaga <[email protected]>
management/server/account.go Outdated Show resolved Hide resolved
management/server/ephemeral.go Outdated Show resolved Hide resolved
management/server/http/peers_handler.go Outdated Show resolved Hide resolved
Comment on lines +84 to +97
err = am.Store.ExecuteInTransaction(ctx, func(transaction Store) error {
groups, err = transaction.GetAccountGroups(ctx, LockingStrengthShare, accountID)
if err != nil {
return err
}

peers, err = transaction.GetAccountPeers(ctx, LockingStrengthShare, accountID)
if err != nil {
return err
}

settings, err = transaction.GetAccountSettings(ctx, LockingStrengthShare, accountID)
return err
})
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure how much effect this transaction will have but should not hurt either

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was waiting for #2957 to measure the performance, but I am already planning to continue passing the account object instead of db calls, since all the methods calling this have access to the account object

management/server/management_proto_test.go Show resolved Hide resolved
Copy link

sonarqubecloud bot commented Dec 9, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
4 New issues
4 New Code Smells (required ≤ 0)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

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.

2 participants