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

Fix whitelist upload batching #612

Merged
merged 2 commits into from
Apr 19, 2024
Merged

Conversation

calebroseland
Copy link
Member

Summary

Fix bug where whitelist uploads would fail if size >100.

CleanShot 2024-04-18 at 12 20 35

Ticket Link

@calebroseland calebroseland added the 2: Dev Review Requires review by a core committer label Apr 18, 2024
@calebroseland calebroseland added this to the v1.12 milestone Apr 18, 2024
Copy link
Member

@JulienTant JulienTant left a comment

Choose a reason for hiding this comment

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

LGTM, just a small remark on the log level

server/api.go Outdated
@@ -807,6 +807,8 @@ func (a *API) updateWhitelist(w http.ResponseWriter, r *http.Request) {
return
}

a.p.API.LogWarn("Whitelist updated", "size", len(ids))
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't this be an info?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, bad copy-paste. Thanks for catching!

if err = s.storeUsersInWhitelist(currentBatch, tx); err != nil {
s.api.LogDebug("Error adding batched users to whitelist", "error", err.Error(), "userIds", currentBatch)
return err
}
clear(currentBatch)
currentBatch = nil
Copy link
Member

@JulienTant JulienTant Apr 18, 2024

Choose a reason for hiding this comment

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

TIL clear keeps the size!

https://go.dev/play/p/a1Y-PEiGOQs

@calebroseland calebroseland added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core committer labels Apr 19, 2024
@calebroseland calebroseland merged commit ad14ade into main Apr 19, 2024
6 of 7 checks passed
@calebroseland calebroseland deleted the fix-whitelist-upload-batching branch April 19, 2024 15:27
calebroseland added a commit that referenced this pull request Apr 19, 2024
sbishel pushed a commit that referenced this pull request Apr 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4: Reviews Complete All reviewers have approved the pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants