Skip to content

Conversation

@drigz
Copy link
Contributor

@drigz drigz commented Nov 28, 2025

Please review the individual commits to separate the refactor from the new flag:

  • refactor: use Config struct for server.go
  • feat: add flag to change request timeout

We've seen outages where a critical request goes from taking slightly less than 60s to a bit more, and it's hard to response to these as the timeout is hard-coded. This should give us the flexibility to change it.

This makes it easier to add parameters without too many positional
arguments to Start().
@drigz drigz requested a review from ensonic November 28, 2025 10:04
@drigz drigz changed the title rodrigoq/timeout flag http-relay-server: Add a flag to set inactive request timeout Nov 28, 2025
Copy link
Contributor

@ensonic ensonic left a comment

Choose a reason for hiding this comment

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

Thanks. As a followup we can use the Option pattern. Then we have the setting directly in the server struct and avoid the long derefs at runtime (for both readability and performance)

@ensonic
Copy link
Contributor

ensonic commented Nov 28, 2025

You'll need to also adjust the in_process_relay_test.go:

src/go/tests/relay/in_process_relay_test.go:51:19: not enough arguments in call to server.NewServer
	have ()
	want (server.Config)
src/go/tests/relay/in_process_relay_test.go:52:22: too many arguments in call to relayServer.Start
	have (int, int)
	want ()

@drigz drigz force-pushed the rodrigoq/timeout-flag branch from 679091b to 47a51ff Compare November 28, 2025 12:36
@drigz drigz force-pushed the rodrigoq/timeout-flag branch from 47a51ff to 6112fe1 Compare November 28, 2025 12:59
@drigz drigz enabled auto-merge (squash) November 28, 2025 15:03
@drigz drigz merged commit a4e6df1 into main Nov 28, 2025
6 checks passed
@drigz drigz deleted the rodrigoq/timeout-flag branch November 28, 2025 15:03
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.

3 participants