Add abort functionality to search #2370
Conversation
Signed-off-by: Andrew W. Harn <[email protected]>
awharn
requested review from
t1m0thyj,
gejohnston,
zFernand0,
traeok,
anaxceron,
jace-roell and
ATorrise
Signed-off-by: Andrew W. Harn <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2370 +/- ##
==========================================
- Coverage 91.27% 91.27% -0.01%
==========================================
Files 636 636
Lines 18083 18096 +13
Branches 3862 3868 +6
==========================================
+ Hits 16506 16517 +11
- Misses 1576 1578 +2
Partials 1 1 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
Signed-off-by: Andrew W. Harn <[email protected]>
| @@ -45,4 +45,7 @@ export interface ISearchOptions { | |||
| /* A function that, if provided, is called with a list of data sets and members that are about to be searched. */ | |||
| /* If true, continue search. If false, terminate search. */ | |||
| continueSearch?: (dataSets: IDataSet[]) => Promise<boolean> | boolean; | |||
|
|
|||
| /* A function that gets called to validate whether or not to abort if a timeout isn't specified */ | |||
| abortSearch?: () => boolean; | |||
There was a problem hiding this comment.
It is unclear how we expect the author of an abortSearch() callback to decide whether to return true or false.
Without that understanding, it is unclear how we expect the search to behave in various scenarios.
Perhaps you could describe the intention in a parking lot or other meeting?
There was a problem hiding this comment.
I updated the comment to try and be more clear about the intention of the callback. In VSCode, there is a concept of a cancellation token, so in that context, the passed in callback will be () => { return token.isCancellationRequested } where that variable is a boolean reflective of if the user pressed the 'cancel' button on the progress bar UI. Effectively, the desired behavior is that the callback is checked before every search, and if the callback suddenly starts to return true, the calling function is requesting that the search operations be aborted immediately, and that further requests to the resource should cease.
There was a problem hiding this comment.
This could also be true for other extending applications, since searching hundreds or thousands of data sets or members could take a very long time, and there is no way for a calling application to stop the inner code running in an asynchronous function once it starts, unless that function provides a method to terminate it early.
There was a problem hiding this comment.
Thanks for the clarification.
Signed-off-by: Andrew W. Harn <[email protected]>
Signed-off-by: Andrew W. Harn <[email protected]>
| @@ -45,4 +45,7 @@ export interface ISearchOptions { | |||
| /* A function that, if provided, is called with a list of data sets and members that are about to be searched. */ | |||
| /* If true, continue search. If false, terminate search. */ | |||
| continueSearch?: (dataSets: IDataSet[]) => Promise<boolean> | boolean; | |||
|
|
|||
| /* A function that gets called to validate whether or not to abort if a timeout isn't specified */ | |||
| abortSearch?: () => boolean; | |||
There was a problem hiding this comment.
Thanks for the clarification.
Signed-off-by: Andrew W. Harn <[email protected]>
Signed-off-by: Andrew W. Harn <[email protected]>
Signed-off-by: Andrew W. Harn <[email protected]>
| @@ -146,6 +146,10 @@ export class Search { | |||
| searchOptions.progressTask.stageName = TaskStage.FAILED; | |||
| searchOptions.progressTask.percentComplete = 100; | |||
| searchOptions.progressTask.statusMessage = "Operation timed out"; | |||
| } else if (searchOptions.abortSearch && searchOptions.abortSearch()) { | |||
There was a problem hiding this comment.
it seems that doing searchOptions.abortSearch?.() works in typescript. 🤯
(at least when using the latest version of ts-node 😋 )
There was a problem hiding this comment.
That is both awesome and terrifying syntax. I can give it a shot.
Signed-off-by: Andrew W. Harn <[email protected]>
|
t1m0thyj
added
the
release-patch
|
Release succeeded for the The following packages have been published:
Powered by Octorelease 🚀 |
What It Does
Adds the ability to specify a function to tell the search functions to stop.
How to Test
Review Checklist
I certify that I have:
Additional Comments