-
Notifications
You must be signed in to change notification settings - Fork 218
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
Enforce nexus request timeout in workflow test suite #1653
Changes from all commits
9028e18
337022e
ad8e20b
eb6f146
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2358,6 +2358,17 @@ func (env *testWorkflowEnvironmentImpl) ExecuteNexusOperation( | |
) int64 { | ||
seq := env.nextID() | ||
taskHandler := env.newTestNexusTaskHandler() | ||
// Use lower case header values to simulate how the Nexus SDK (used internally by the "real" server) would transmit | ||
// these headers over the wire. | ||
nexusHeader := make(map[string]string, len(params.nexusHeader)) | ||
for k, v := range params.nexusHeader { | ||
nexusHeader[strings.ToLower(k)] = v | ||
} | ||
params.nexusHeader = nexusHeader | ||
// The real server allows requests to take up to 10 seconds, mimic that behavior here. | ||
// Note that if a user sets the Request-Timeout header, it gets overridden. | ||
params.nexusHeader[strings.ToLower(nexus.HeaderRequestTimeout)] = "10s" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't the real server behaviour the minimum of 10s or the remaining schedule to close? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point. Not at the moment, but we should enforce that eventually. |
||
|
||
handle := &testNexusOperationHandle{ | ||
env: env, | ||
seq: seq, | ||
|
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.
We also set the request timeout for the cancel operation.
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.
Yes, this will set that timeout for all "requests".