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

support shortcircuit in Request Modifiers #731

Closed

Conversation

minizilla
Copy link

Support shortcircuit in Request Modifiers by allowing to returns ResponseWrapper. Close #730, the issue doesn't have much comment but since this is trivial, I create a PR regardless.

@minizilla minizilla force-pushed the enhance-req-resp-modifier branch from 0c96f37 to c93e70d Compare August 27, 2024 09:44
Signed-off-by: Billy Zaelani Malik <[email protected]>
@minizilla minizilla force-pushed the enhance-req-resp-modifier branch from c93e70d to 80b1009 Compare August 27, 2024 09:48
@kpacha
Copy link
Member

kpacha commented Dec 4, 2024

Hi @minizilla ,

Thanks for the contribution. I think that the feature makes a lot of sense but I don't think this solution is the optimal one. Instead of extending the semantics of the request modifier plugin, I'd create a new type of plugin (let's call it proxy plugin for now) that exposes a signature similar to the proxy.Proxy type/interface. With this implementation, we could solve 2 different requirements: being able to abort the pipe execution and having a shared context where the request and the related response are clearly available in a single function.

Ignoring all the boilerplate required for declaring the type and registering the factory, I think that proxy plugin could have a shape like this:

type ProxyWrapper func(context.Context, RequestWrapper) (ResponseWrapper, error)

func (registerer) registerProxyPlugin(globalCtx context.Context, cfg map[string]interface{}, next ProxyWrapper) ProxyWrapper {
	// check the config and, if the plugin shouldn't be injected, just return `next`
 	return func(ctx context.Context, r RequestWrapper) (ResponseWrapper, error){
	 	// plugin logic here. you could just return a response wrapper and an error in order to abort the pipe
	 	// or just delegate to `next` (maybe after altering the request) and use its results to generate what
	 	// the plugin will return
	 	return next(ctx, r)
 	}
}

This is a quick sketch and maybe the signatures will require some modifications (like using interface{} in order to avoid sharing types) but I think it's a good starting point for the discussion.

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.

Enable Request Modifiers to short circuit and returning ResponseWrapper
2 participants