Provide a client method to bypass server side timeout on upload #20419
Conversation
openshift-ci-robot
added
the
size/M
openshift-ci-robot
added
the
approved
| Resource("imagestreamimports"). | ||
| Body(imageStreamImport). | ||
| // this instructs the api server to allow our request to take up to an hour - chosen as a high boundary | ||
| Timeout(time.Hour). |
There was a problem hiding this comment.
why not just make this an argument to the function? especially since the function name is currently a lie.
also where's the generator code change that yields this?
There was a problem hiding this comment.
Because we don't want end users setting arbitrary timeouts. We define what "no timeout" means behind the interface. End clients don't get to set it.
There was a problem hiding this comment.
There is no generator code change. This is an expansion, which are side car methods that you write and get slotted in to the interfaces.
There was a problem hiding this comment.
An expansion that we will have to carry over when we move the image controllers to external :-)
There was a problem hiding this comment.
@smarterclayton if there is just one place where we actually use this expansion, can't we just call the client directly with Timeout in that place? (instead of having one-time-use expansion)
There was a problem hiding this comment.
can't we just call the client directly with Timeout in that place?
the client doesn't expose the timeout field that i saw, so no.
smarterclayton
force-pushed
the
timeout_of_import
branch
from
219b1fb to
6dc1369
Compare
The last import had no change, so we were relying on the older, shorter timeout. Instead, we want to explicitly trigger this error.
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bparees, smarterclayton The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
Fixes one part of https://bugzilla.redhat.com/show_bug.cgi?id=1608410
The import we were doing took longer than 30s in 3.11. In 3.10 it was only occasionally taking that long. ImageStreamImport is a "long running call", so when we perform it we need to allow it to run slightly longer for some use cases.
In general, the product has to support imports longer than 30s.