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

VCL: Restrict scope of req.is_hit{miss,pass} #4206

Merged
merged 3 commits into from
Oct 7, 2024

Conversation

walid-git
Copy link
Member

Reading the values elsewhere doesn't make much sense.

@dridi
Copy link
Member

dridi commented Oct 2, 2024

Should we also include vcl_synth for both? That's a possible transition after vcl_miss and vcl_pass.

@walid-git
Copy link
Member Author

Should we also include vcl_synth for both? That's a possible transition after vcl_miss and vcl_pass.

I was hesitant about that one and thought I would wait for opinions, there is also the possibility to go to pass from miss, so we may also want to allow is_hitmiss in vcl_pass.

@AlveElde
Copy link
Contributor

AlveElde commented Oct 2, 2024

I would add vcl_synth, as it is fairly common to share logic between vcl_synth and vcl_deliver with a subroutine called from both places.

@dridi
Copy link
Member

dridi commented Oct 2, 2024

You will also end up in vcl_synth if your fetch failed.

Checking the value of req.is_hitpass in other places doesn't make sense
Checking the value of req.is_hitmiss in other places doesn't make sense
@walid-git walid-git force-pushed the is_hit_misspass_scope branch from 84942f2 to e4f3574 Compare October 2, 2024 15:30
@walid-git
Copy link
Member Author

Added vcl_pass for is_hitmiss and vcl_synth for both.

@walid-git
Copy link
Member Author

bugwash: OK after adding to release notes.

@walid-git walid-git merged commit 305be79 into varnishcache:master Oct 7, 2024
10 of 11 checks passed
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.

4 participants