-
Notifications
You must be signed in to change notification settings - Fork 40
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
[nexus-test-utils] set 60s timeouts for each init step #4789
[nexus-test-utils] set 60s timeouts for each init step #4789
Conversation
Created using spr 1.3.5
Created using spr 1.3.5
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neat, strongly in favor of the better visibility during test setup!
@@ -604,18 +636,21 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> { | |||
dns_config_client.dns_config_put(&dns_config).await.expect( | |||
"Failed to send initial DNS records to internal DNS server", | |||
); | |||
dns_config | |||
self.dns_config = Some(dns_config); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just to avoid a move
between setup steps, yeah?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this pattern, we no longer have a way to return a value that the next step can use. This is the easiest way to work around that (and follows the pattern the rest of the builder uses anyway).
I ran into something similar with the update-engine and came up with the design to use step handles that can be passed around, but only resolved once steps start executing. For example,
omicron/update-engine/examples/update-engine-basic/main.rs
Lines 308 to 310 in 45b6651
let buf_list = download_handle.into_value(cx.token()).await; | |
let temp_dirs = | |
temp_dirs_handle.into_value(cx.token()).await; |
Created using spr 1.3.5
In #4779 we're tracking what appears to be a ClickHouse initialization failure
during Nexus startup. Set a timeout of 60s for each step in the initialization
process.
These steps should usually not take more than 5 seconds each, so 60s is a
really comfortable buffer.