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

move logic of VDI_Resolve() to VRT as VRT_DirectorResolve() #2680

Closed
wants to merge 6 commits into from

Conversation

nigoroll
Copy link
Member

VDI_Resolve() remains for error handling for calls from VDI_GetHdr() and VDI_Http1Pipe()

@nigoroll nigoroll force-pushed the VRT_DirectorResolve branch 2 times, most recently from 81fa673 to ba5ee1b Compare May 24, 2018 10:31
@nigoroll nigoroll force-pushed the VRT_DirectorResolve branch 2 times, most recently from f78eea3 to 4683e30 Compare May 31, 2018 12:42
@nigoroll nigoroll force-pushed the VRT_DirectorResolve branch 4 times, most recently from 0460391 to 3b17f4e Compare June 11, 2018 08:30
@nigoroll
Copy link
Member Author

FTR, I had accidentally pushed a bad commit in the meantime, now force-pushed the branch as it should be

@nigoroll nigoroll force-pushed the VRT_DirectorResolve branch 3 times, most recently from a74b392 to 30a5a1f Compare June 28, 2018 08:17
@nigoroll nigoroll force-pushed the VRT_DirectorResolve branch 2 times, most recently from 3c91cb3 to 1a140c7 Compare August 28, 2018 21:33
@nigoroll nigoroll force-pushed the VRT_DirectorResolve branch 2 times, most recently from e533964 to e0999d0 Compare September 3, 2018 10:35
@nigoroll nigoroll force-pushed the VRT_DirectorResolve branch 2 times, most recently from 9e66561 to d31d6ca Compare October 9, 2018 10:57
@nigoroll nigoroll force-pushed the VRT_DirectorResolve branch 2 times, most recently from 49c5da5 to 575577a Compare November 1, 2018 07:58
@nigoroll nigoroll force-pushed the VRT_DirectorResolve branch 2 times, most recently from 81e75e9 to c6a4ab9 Compare November 6, 2018 10:26
@nigoroll nigoroll force-pushed the VRT_DirectorResolve branch 3 times, most recently from 678f122 to d4370af Compare March 10, 2020 13:19
@nigoroll nigoroll force-pushed the VRT_DirectorResolve branch 3 times, most recently from 7909d35 to cc481f3 Compare April 9, 2020 15:25
The use case is eager resolution of directors in vmods, for example to
determine if a director would resolve to a particular backend.

The functionality is identical to VDI_Resolve(), except that no state
in the busyobj is modified.

VDI_Resolve() remains for error handling for calls from VDI_GetHdr()
and VDI_Http1Pipe()
@Dridi rightly pointed out that, in the for loop condition, we accessed
d->vdir->methods before asserting that d->vdir is non-NULL.

Simplify and clean up for clarity.

This code was basically just copied from existing code, so the issue
addressed here exited before the previous change to introduce
VRT_DirectorResolve.
bsdphk added a commit to bsdphk/varnish-cache that referenced this pull request May 11, 2020
@bsdphk
Copy link
Contributor

bsdphk commented May 11, 2020

Please see #3311 for an alternative implementation using type-specific methods.

nigoroll pushed a commit that referenced this pull request May 13, 2020
This is an alternative implementation of #2680
@nigoroll
Copy link
Member Author

merged #3311 instead

@nigoroll nigoroll closed this May 13, 2020
nigoroll added a commit to nigoroll/varnish-cache that referenced this pull request May 13, 2020
recover a minor improvement from varnishcache#2680:

This commit is to emit slightly different error messages to
differentiate between the cases "no backend set in vcl" and
"none returned from the director".

Includes minor polishing

Ref varnishcache#2860 varnishcache#3311
nigoroll added a commit to nigoroll/varnish-cache that referenced this pull request May 14, 2020
recover a minor improvement from varnishcache#2680:

This commit is to emit slightly different error messages to
differentiate between the cases "no backend set in vcl" and
"none returned from the director".

Includes minor polishing

Ref varnishcache#2860 varnishcache#3311
nigoroll added a commit to nigoroll/varnish-cache that referenced this pull request May 19, 2020
recover a minor improvement from varnishcache#2680:

This commit is to emit slightly different error messages to
differentiate between the cases "no backend set in vcl" and
"none returned from the director".

Includes minor polishing

Ref varnishcache#2860 varnishcache#3311
nigoroll added a commit to nigoroll/varnish-cache that referenced this pull request May 24, 2020
recover a minor improvement from varnishcache#2680:

This commit is to emit slightly different error messages to
differentiate between the cases "no backend set in vcl" and
"none returned from the director".

Includes minor polishing

Ref varnishcache#2860 varnishcache#3311
nigoroll added a commit to nigoroll/varnish-cache that referenced this pull request Jun 10, 2020
recover a minor improvement from varnishcache#2680:

This commit is to emit slightly different error messages to
differentiate between the cases "no backend set in vcl" and
"none returned from the director".

Includes minor polishing

Ref varnishcache#2860 varnishcache#3311
nigoroll added a commit to nigoroll/varnish-cache that referenced this pull request Jun 23, 2020
recover a minor improvement from varnishcache#2680:

This commit is to emit slightly different error messages to
differentiate between the cases "no backend set in vcl" and
"none returned from the director".

Includes minor polishing

Ref varnishcache#2860 varnishcache#3311
nigoroll added a commit to nigoroll/varnish-cache that referenced this pull request Jun 23, 2020
Ref
varnishcache#3315 (comment)

OKed by phk on irc.

VRT_DirectorResolve() was the name originally proposed in varnishcache#2680, which
was renamed in varnishcache#3311 and accepted by me to get ahead. Not that Dridi
prefers the original name and phk goes along, we change it back before
the next release.
@nigoroll nigoroll deleted the VRT_DirectorResolve branch June 23, 2020 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a=need bugwash a=RunByUPLEX This PR is being run by UPLEX in production
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants