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

Support uploading stash/filter from multiple block types #22884

Merged
merged 6 commits into from
Nov 25, 2024

Conversation

KevinMind
Copy link
Contributor

@KevinMind KevinMind commented Nov 22, 2024

Relates to: mozilla/addons#15014

Description

Adds support for uploading filters and stashes simultaneously as well as supporting soft/hard filters.

Context

Supporting #22828

Testing

Setup

  • Setup a local remote server here
  • Set the base replace threshold to a low number (so you can trigger re-uploading of filters without creating a bunch of blocks)
  • enable enable-soft-blocking and blocklist_mlbf_submit waffle switch

src/olympia/constants/blocklist.py

BASE_REPLACE_THRESHOLD = 2

See the test scenarios

from olympia.blocklist.models import BlockType
from olympia.amo.tests import addon_factory, block_factory, version_factory

def _blocked_addon(block_type=BlockType.BLOCKED, **kwargs):
    addon = addon_factory(**kwargs)
    block = block_factory(
        guid=addon.guid, updated_by=user, block_type=block_type
    )
    return addon, block

user = UserProfile.objects.first()

Now you can call the _blocked_addon method to create an addon with block/version of the specified type.

Ex:

_blocked_addon(block_type=BlockType.BLOCKED)
_blocked_addon(block_type=BlockType.BLOCKED)
_blocked_addon(block_type=BlockType.SOFT_BLOCKED)

This PR will not change any of the existing behavior so you expect to see a new stash record if >0Replace threshold changes, also if new filter expect previous stashes to be deleted.

Important @willdurand you need to remove the soft block config key from admin or the deletion will depend on arbitrary soft block times which are probably wrong

Force recreate

First force recreate the filter

./manage.py cron upload_mlbf_to_remote_settings force_base=True

Expect only an attachment record, regardless of what was there before.

Create a stash

Create one new block

_blocked_addon(block_type=BlockType.BLOCKED file_kw={'is_signed': True})
./manage.py cron upload_mlbf_to_remote_settings

Expect a stash record uploaded

Repeat, expect another stash record

Repeat, now expect a new filter and all stashes deleted.

You can create as many soft blocks at any point in this flow and it should have zero impact.

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 changed the base branch from soft-block-bloom-filter-filter to master November 22, 2024 14:34
@KevinMind KevinMind force-pushed the soft-block-upload-task branch from 53e94aa to 7c7e5b5 Compare November 22, 2024 14:41
@KevinMind KevinMind marked this pull request as ready for review November 22, 2024 14:44
@KevinMind KevinMind force-pushed the soft-block-upload-task branch from 7c7e5b5 to 80d0bb9 Compare November 22, 2024 15:14
@KevinMind KevinMind requested a review from willdurand November 22, 2024 15:14
@KevinMind KevinMind force-pushed the soft-block-upload-task branch from 80d0bb9 to 0684796 Compare November 22, 2024 15:30
@KevinMind
Copy link
Contributor Author

Screen.Recording.2024-11-22.at.17.01.37.mov

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.

Thanks, I noticed a bug with the deletion of previous records but apart from that, this is looking solid.

src/olympia/blocklist/tasks.py Outdated Show resolved Hide resolved
src/olympia/blocklist/tests/test_cron.py Outdated Show resolved Hide resolved
src/olympia/blocklist/tests/test_cron.py Outdated Show resolved Hide resolved
src/olympia/blocklist/tests/test_tasks.py Outdated Show resolved Hide resolved
src/olympia/blocklist/tests/test_tasks.py Outdated Show resolved Hide resolved
src/olympia/blocklist/tests/test_tasks.py Outdated Show resolved Hide resolved
src/olympia/blocklist/tests/test_tasks.py Outdated Show resolved Hide resolved
src/olympia/constants/blocklist.py Show resolved Hide resolved
Co-authored-by: William Durand <will+git@drnd.me>
@KevinMind KevinMind force-pushed the soft-block-upload-task branch from 400fa6f to d8f6551 Compare November 25, 2024 12:32
@KevinMind KevinMind requested a review from willdurand November 25, 2024 12:32
… config_keys before completing the session.. if any error raises we have updated config keys, but not updated files in remote settings which could be a critical bug
@KevinMind KevinMind force-pushed the soft-block-upload-task branch from d8f6551 to 5c9e6f0 Compare November 25, 2024 13:18
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.

Mostly nits, and a suggestion to compute the oldest base ID. The rest looks good, thanks.

src/olympia/blocklist/tasks.py Outdated Show resolved Hide resolved
src/olympia/blocklist/tasks.py Outdated Show resolved Hide resolved
src/olympia/blocklist/tasks.py Outdated Show resolved Hide resolved
src/olympia/blocklist/tasks.py Outdated Show resolved Hide resolved
src/olympia/blocklist/tasks.py Outdated Show resolved Hide resolved
src/olympia/blocklist/tasks.py Outdated Show resolved Hide resolved
src/olympia/blocklist/tasks.py Outdated Show resolved Hide resolved
src/olympia/blocklist/tasks.py Outdated Show resolved Hide resolved
@KevinMind KevinMind requested a review from willdurand November 25, 2024 14:23
@KevinMind KevinMind force-pushed the soft-block-upload-task branch 2 times, most recently from 2488f60 to 9efc622 Compare November 25, 2024 14:33
Co-authored-by: William Durand <will+git@drnd.me>
@KevinMind KevinMind force-pushed the soft-block-upload-task branch from 9efc622 to 256e667 Compare November 25, 2024 15:02
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+wc, thank you!

src/olympia/blocklist/tasks.py Outdated Show resolved Hide resolved
src/olympia/blocklist/tasks.py Outdated Show resolved Hide resolved
src/olympia/blocklist/tasks.py Outdated Show resolved Hide resolved
src/olympia/blocklist/tasks.py Outdated Show resolved Hide resolved
src/olympia/blocklist/tasks.py Outdated Show resolved Hide resolved
src/olympia/blocklist/tasks.py Outdated Show resolved Hide resolved
src/olympia/blocklist/tasks.py Outdated Show resolved Hide resolved
src/olympia/blocklist/tasks.py Outdated Show resolved Hide resolved
src/olympia/blocklist/tasks.py Outdated Show resolved Hide resolved
src/olympia/blocklist/tasks.py Outdated Show resolved Hide resolved
Co-authored-by: William Durand <will+git@drnd.me>
@KevinMind KevinMind merged commit f148cd3 into master Nov 25, 2024
31 checks passed
@KevinMind KevinMind deleted the soft-block-upload-task branch November 25, 2024 16:33
chrstinalin pushed a commit that referenced this pull request Jan 10, 2025
* Support uploading stash/filter from multiple block types

* Apply suggestions from code review

Co-authored-by: William Durand <will+git@drnd.me>

* Fix ordering of complete_session: (this has a new bug that we set the config_keys before completing the session.. if any error raises we have updated config keys, but not updated files in remote settings which could be a critical bug

* Better, more explicit tests and then make them all pass

* Apply suggestions from code review

Co-authored-by: William Durand <will+git@drnd.me>

* Apply suggestions from code review

Co-authored-by: William Durand <will+git@drnd.me>

---------

Co-authored-by: William Durand <will+git@drnd.me>
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