Skip to content
This repository has been archived by the owner on May 5, 2023. It is now read-only.

0.44 Fix prioritization #264

Merged
merged 13 commits into from Sep 3, 2021
Merged

0.44 Fix prioritization #264

merged 13 commits into from Sep 3, 2021

Conversation

FedericoCeratto
Copy link
Contributor

@FedericoCeratto FedericoCeratto commented Aug 26, 2021 •
edited

  • Bugfix
  • Add ability to disable an URL or category completely by setting a very low negative priority
  • Improve testing

@FedericoCeratto FedericoCeratto marked this pull request as ready for review August 26, 2021 17:10
hellais
hellais reviewed Sep 2, 2021
View reviewed changes
newapi/tests/integ/test_prioritization_nodb.py
): Path("url_priorities_us.json"),
}


Copy link
Member

Choose a reason for hiding this comment

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

Since these tests are based on deterministic database dumps, perhaps we can also add a check which verifies that the sorting of the returned URLs matches the expected result (i.e. the priorities are in fact being respected)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

+1

hellais
hellais approved these changes Sep 2, 2021
View reviewed changes
Copy link
Member

@hellais hellais left a comment

Choose a reason for hiding this comment

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

I reviewed the diff and it looks good to me. I left a couple of comments for improvements that can either be done as part of this PR or as future work.

@FedericoCeratto FedericoCeratto mentioned this pull request Sep 3, 2021
Closed
@FedericoCeratto FedericoCeratto merged commit c5f1623 into master Sep 3, 2021
@FedericoCeratto FedericoCeratto deleted the fix-prio branch September 3, 2021 16:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@hellais hellais hellais approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@FedericoCeratto @hellais