add sessionspaces perparation gate to graph-proxy - #1666
iamvigneshwars wants to merge 6 commits into
Conversation
| @@ -274,8 +274,8 @@ impl WorkflowTemplatesMutation { | |||
| ) -> anyhow::Result<Workflow> { | |||
There was a problem hiding this comment.
Not for this PR but for future consideration: we may want to rethink this API.
With the new dynamic sessionspaces users may end up waiting for a response for 150 seconds.
Clients may give up and timeout before that.
I don't know if that it going to cause problems.
There was a problem hiding this comment.
Yeah I get you concern. But usually it happens only once when a users creates a new workflow in a new session or a dormant sessionspace. Practically, provisioning the namespace and required resources takes around 2 to 5 seconds. Once the namespace has been created and the SessionSpace is active, subsequent workflow requests shouldn't have any noticeable additional latency.
| /// Forward the caller's token and wait for preparation, failing closed on any error. | ||
| pub async fn prepare_session(api_url: &Url, namespace: &str, token: Option<&str>) -> Result<()> { | ||
| let failure = |status: StatusCode, message: &str| { | ||
| anyhow!("SessionSpace preparation failed for {namespace} ({status}): {message}") |
There was a problem hiding this comment.
I think that these error messages may be surfaced to users via graphql errors? If so, internal jargon: "SessionSpace preparation failed " may be confusing to the user.
There was a problem hiding this comment.
Good catch, I have updated it.
| # Submission tests build very large debug futures; the default test-thread | ||
| # stack overflows. 16 MiB leaves headroom for future growth. | ||
| env: | ||
| RUST_MIN_STACK: "16777216" |
There was a problem hiding this comment.
do we understand why the futures are so large? If this only affects test/debug builds then this seems reasonable. I just want to make sure we're not masking an underlying issue in the production async code that could cause problems later.
There was a problem hiding this comment.
I think these are debug only. The submission tests deserialise the generated argo openAPI types, so in debug builds the serde frames exceed 2 MB default test-thread stack. Release builds optimise these frames, so this is just a test harness.
9761473 to
5e831df
Compare
This branch has not been deployed
2 participants
No description provided.