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

Remove time inefficient loop in not_blocked_items #22862

Merged
merged 1 commit into from
Nov 18, 2024
Merged

Conversation

KevinMind
Copy link
Contributor

@KevinMind KevinMind commented Nov 18, 2024

Relates to: mozilla/addons#15170

Description

Removes time inefficient loop in get_not_blocked_items by filtering from a once created set instead of an inlined list

Context

This patch adds a slow test 20 seconds that can be isolated from the rest of our tests in a follow up PR

Testing

CI passes, this PR will be verified on staging as well.

Checklist

  • Add #ISSUENUM at the top of your PR to an existing open issue in the mozilla/addons repository.
  • Successfully verified the change locally.
  • The change is covered by automated tests, or otherwise indicated why doing so is unnecessary/impossible.
  • Add before and after screenshots (Only for changes that impact the UI).
  • Add or update relevant docs reflecting the changes made.

@KevinMind KevinMind requested a review from willdurand November 18, 2024 10:56
@KevinMind KevinMind changed the base branch from remove-all-quadratics to master November 18, 2024 10:56
src/olympia/blocklist/mlbf.py Outdated Show resolved Hide resolved
src/olympia/blocklist/mlbf.py Outdated Show resolved Hide resolved
@KevinMind KevinMind requested a review from willdurand November 18, 2024 11:25
Copy link
Member

@willdurand willdurand left a comment

Choose a reason for hiding this comment

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

Could you squash your commits to get a single commit in this PR? I know GitHub will do it eventually but it'd be easier to navigate the changes locally.

src/olympia/blocklist/mlbf.py Outdated Show resolved Hide resolved
@KevinMind
Copy link
Contributor Author

Ok @willdurand let's do it your way. I have things to say about your feedback but PR comments is not the place, especially when the goal here is to "unbreak prod on the short term"

Remove test

Revert reuse of reusable function
Copy link
Member

@willdurand willdurand left a comment

Choose a reason for hiding this comment

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

r+, thanks

@KevinMind KevinMind merged commit 4d0a7a2 into master Nov 18, 2024
31 checks passed
@KevinMind KevinMind deleted the fix-prod branch November 18, 2024 13:32
KevinMind added a commit that referenced this pull request Nov 18, 2024
Remove test

Revert reuse of reusable function
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