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

Added init_downstream_modules phase allowing modules to be set up before startup #284

Closed
wants to merge 1 commit into from

Conversation

palant
Copy link
Contributor

@palant palant commented Jun 15, 2024

This addresses a roadblock I’ve hit trying to make the new downstream modules mechanism work for me: these cannot be configured from within the HttpProxy instance. The configuration has to happen after http_proxy_service has been called, but at this point the HttpProxy instance has been moved into the service and is no longer accessible. In order to adjust the compression level for example some code outside the actual handler would have to mess with the service configuration which is a significant logic break.

In case of compression this can be addressed by adjusting compression level in early_request_filter. When the handler wants to add its own module however (helps remove some work-arounds I’ve added before), this option is rather suboptimal.

So I’ve added an init_downstream_module phase where HttpProxy can adjust modules to its liking.

@palant
Copy link
Contributor Author

palant commented Jun 15, 2024

When the handler wants to add its own module however (helps remove some work-arounds I’ve added before), this option is rather suboptimal.

Actually, I can see now that the module list is no longer mutable during early_request_filter, so adding modules isn’t even possible there. Hooking into the server initialization is really the only way.

@eaufavor
Copy link
Member

You can access and mutate the modules here. Be this PR also provides a nicer API.

let mut proxy = http_proxy_service(...);
proxy.app_logic_mut().unwrap().downstream_modules

@eaufavor eaufavor added the enhancement New feature or request label Jun 20, 2024
@palant
Copy link
Contributor Author

palant commented Jun 21, 2024

You can access and mutate the modules here.

I know. As I said, doing this via some code outside the handler is possible but a significant logic break.

@eaufavor eaufavor self-assigned this Jun 21, 2024
@eaufavor eaufavor added the WIP We are working on this feature internally label Jun 21, 2024
drcaramelsyrup pushed a commit that referenced this pull request Jun 28, 2024
@eaufavor eaufavor added Accepted This change is accepted by us and merged to our internal repo and removed WIP We are working on this feature internally labels Jun 28, 2024
drcaramelsyrup pushed a commit that referenced this pull request Jun 28, 2024
@drcaramelsyrup
Copy link
Contributor

We've brought this into main as dda7bec. Thanks!

escoffier pushed a commit to escoffier/pingora that referenced this pull request Sep 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accepted This change is accepted by us and merged to our internal repo enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants