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

HSH: Limit the number of non matching variants evaluated against the ban list during lookup #4253

Merged
merged 4 commits into from
Jan 22, 2025

Conversation

walid-git
Copy link
Member

@walid-git walid-git commented Jan 15, 2025

This is an implementation of the solution suggested by @bsdphk in #4236 (comment)

It adds a parameter to limit the number of possibly non matching variants that we evaluate against the ban list during a lookup.

Leaving this as a draft until we agree on the parameter name.

Refs: #4236

@nigoroll nigoroll mentioned this pull request Jan 15, 2025
8 tasks
@nigoroll
Copy link
Member

after bugwash discussion:

  • we settled on ban_any_variant for the name
  • the default should be changed to 0 with a major release (added a reminder to 8.0 release checklist #3352)
  • the parameter should be copied to a local variable for consistency, otherwise both ban_checks related conditions or none could become true

@walid-git
Copy link
Member Author

  • we settled on ban_any_variant for the name
  • the default should be changed to 0 with a major release (added a reminder to 8.0 release checklist #3352)
  • the parameter should be copied to a local variable for consistency, otherwise both ban_checks related conditions or none could become true

Also added a paragraph in purging.rst about the new ban_any_variant parameter.

@walid-git walid-git marked this pull request as ready for review January 15, 2025 16:08
@walid-git
Copy link
Member Author

bugwash: make docs clearer

@walid-git
Copy link
Member Author

Added 744e840. @bsdphk let me know if this is clearer now.

@nigoroll
Copy link
Member

I was not addressed, but I think the document improvement is very good.

@walid-git walid-git merged commit c52abc1 into varnishcache:master Jan 22, 2025
1 of 10 checks passed
@walid-git walid-git mentioned this pull request Jan 22, 2025
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.

3 participants