Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[Task]: Migrate from BlockVersion.soft to BlockVersion.block_type #15111

Closed
1 task
diox opened this issue Oct 25, 2024 · 7 comments · Fixed by mozilla/addons-server#22811 · May be fixed by mozilla/addons-server#22821
Closed
1 task

[Task]: Migrate from BlockVersion.soft to BlockVersion.block_type #15111

diox opened this issue Oct 25, 2024 · 7 comments · Fixed by mozilla/addons-server#22811 · May be fixed by mozilla/addons-server#22821
Assignees
Labels
qa:not_needed repository:addons-server Issue relating to addons-server
Milestone

Comments

@diox
Copy link
Member

diox commented Oct 25, 2024

Description

#15012 introduced soft-blocks by adding a soft BooleanField to BlockVersion.

We debated switching to a block_type IntegerField instead (or probably a PositiveSmallIntegerField) and initially resisted, in part because the migration is annoying/costly, but it has since become clear this will make our lives easier, by allowing us to have constants/choices that are simpler to deal with (we can't have Choices with booleans).

Acceptance Criteria

Milestones/checkpoints

Checks

  • If I have identified that the work is specific to a repository, I have removed "repository:addons-server" or "repository:addons-frontend"

┆Issue is synchronized with this Jira Task

@diox
Copy link
Member Author

diox commented Oct 29, 2024

If there was a django field for TINYINT we could use that... Maybe worth writing our own ? That would make the migration slightly faster and take less space for the future.

@diox
Copy link
Member Author

diox commented Oct 29, 2024

If there was a django field for TINYINT we could use that... Maybe worth writing our own ? That would make the migration slightly faster and take less space for the future.

With some luck it's just a couple lines of code to change since we already have a DatabaseWrapper / DatabaseOperations with custom data_types / integer_field_ranges / cast_data_types defined...

@KevinMind
Copy link
Contributor

Question @diox would it be possible to migrate from PositiveSmallIntegerField to a custom TINYINT field in the future? I would prefer not to add a new field, especially a custom rolled one unless we have known/serious performance issues or if we think the migration from A -> B -> C is like wayyyy higher than that of A -> C.

Wdyt?

@diox
Copy link
Member Author

diox commented Oct 30, 2024

I would prefer avoiding migrating a table that has millions of rows over and over. The custom Tinyint field should be easy to do, we already have the basic things set up to do this (src/olympia/core/db/mysql/base.py) as we already have custom db fields (src/olympia/amo/fields.py)

@KevinMind
Copy link
Contributor

@diox looking at the bloom filter and it's kind of not very much fun to try continuing on that without this so I opened a PR just now.. I see you've assigned @bakulf maybe could be used as a reference but I think having the dango integer choices field is my main ask at this point.

class BlockType(models.IntegerChoices):
    BLOCKED = 0, _('Blocked')
    SOFT_BLOCKED = 1, _('Restricted')

If this class exists and is then used by both models, then I don't really care about any other details underlying as I can rely on that enum to build out the block_type based filtering logic. wdyt?

@alexandruschek
Copy link

Hello, can you please help me with some information, I don't understand exactly what I need to check. Thank you very much!

@bakulf
Copy link

bakulf commented Nov 6, 2024

@alexandruschek this task cannot be tested. This is just an internal code change.

# for free to join this conversation on GitHub. Already have an account? # to comment