-
Notifications
You must be signed in to change notification settings - Fork 2k
feat(client-core): Use throwContinueWait parameter in cubeSql method
#10461
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
base: master
Are you sure you want to change the base?
Changes from all commits
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 |
|---|---|---|
|
|
@@ -379,7 +379,7 @@ class CubeApi { | |
| body.error = text; | ||
| } | ||
|
|
||
| if (body.error === 'Continue wait') { | ||
| if (body.error?.includes('Continue wait')) { | ||
| await checkMutex(); | ||
| if (options?.progressCallback) { | ||
| options.progressCallback(new ProgressResult(body as ProgressResponse)); | ||
|
|
@@ -742,6 +742,7 @@ class CubeApi { | |
| signal: options?.signal, | ||
| fetchTimeout: options?.timeout, | ||
| baseRequestId: options?.baseRequestId, | ||
| throwContinueWait: true, | ||
|
Member
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. Like this, it's a breaking change, right? I believe we've agreed to let users configure this parameter by passing an explicit option, and then we'll make it
Member
Author
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. It's not a breaking change since this is a parameter passed by the client and client is designed to handle continue wait as it does with |
||
| }; | ||
|
|
||
| if (options?.cache) { | ||
|
|
||
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.
Let's not do that, because potential it can be a part of error message that are not continue wait.
Uh oh!
There was an error while loading. Please reload this page.
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.
The reason for this change is that we throw this
Continue waitfrom backend-native asCubeError::internalwhich leads to the error having extra prefixes likeExternal error:. I'm open to suggestions fixing the prefixes here but backend-native itself uses the same.containsmethod for testingcontinue waitso I felt like this brings them in line.