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

http proxy: allow multiple listeners #949

Merged
merged 2 commits into from
Nov 8, 2024
Merged

Conversation

mmatczuk
Copy link
Contributor

@mmatczuk mmatczuk commented Nov 7, 2024

Add HTTPProxyConfig.ExtraListeners and refactor code to work with multiple listeners.

http_proxy.go Outdated
Comment on lines 533 to 544
g.Go(func() error {
return srv.Serve(l)
})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^C2024/11/07 14:41:01.894475 [proxy] [INFO] closing down proxy
2024/11/07 14:41:01.894515 [proxy] [INFO] waiting for connections to close
2024/11/07 14:41:01.894522 [proxy] [INFO] all connections closed
2024/11/07 14:41:01.894584 [ERROR] fatal error exiting: http: Server closed
exit status 1

Closing a proxy in a handler mode results in a fatal error.

http_proxy.go Outdated Show resolved Hide resolved
@Choraden
Copy link
Contributor

Choraden commented Nov 7, 2024

LGTM, personally I would go with a wrapper over a list of listeners that returns any accepted connection from the underlying slice, but this is even better!

Left a proposition on adding "named" listeners.

@mmatczuk
Copy link
Contributor Author

mmatczuk commented Nov 8, 2024

MultiListener is complicated its main disadvantage is that you accept a connection and then send it to worker. It's not guaranteed there is a worker available. It could be done but the standard interfaces are not well suited for that.

@mmatczuk
Copy link
Contributor Author

mmatczuk commented Nov 8, 2024

@Choraden PTAL

Add HTTPProxyConfig.ExtraListeners and refactor code to work with multiple listeners.
@mmatczuk
Copy link
Contributor Author

mmatczuk commented Nov 8, 2024

Squashed some commits and rebased.

@mmatczuk mmatczuk merged commit b8ed0f2 into main Nov 8, 2024
6 checks passed
@mmatczuk mmatczuk deleted the mmt/run_with_listener branch November 8, 2024 12:29
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.

2 participants