-
Notifications
You must be signed in to change notification settings - Fork 64
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
Add aborting to CoreGetFeatures rpcManager call #1874
Conversation
d92ebdd
to
7975c87
Compare
Codecov Report
@@ Coverage Diff @@
## main #1874 +/- ##
=======================================
Coverage 62.56% 62.56%
=======================================
Files 484 484
Lines 22707 22711 +4
Branches 5152 5153 +1
=======================================
+ Hits 14206 14210 +4
Misses 8231 8231
Partials 270 270
Continue to review full report at Codecov.
|
plugins/linear-genome-view/src/LinearGenomeView/components/SequenceDialog.tsx
Outdated
Show resolved
Hide resolved
0e33521
to
dadca07
Compare
@garrettjstevens any opinion on the opts parameter being explicitly separate? IIRC this is not how some of our other RPC work, and instead the entire args gets passed in as opts |
@cmdcolin Good point, now that I think about it, |
dadca07
to
cd696ed
Compare
cd696ed
to
72a41ae
Compare
Tried adding GMOD/indexedfasta-js#57 but it did not yet achieve aborting when closing the dialog. Aborting is hard as usual |
ad32d85
to
dd6b510
Compare
…uenceDialog.tsx Co-authored-by: Garrett Stevens <stevens.garrett.j@gmail.com>
dd6b510
to
b5c5ba0
Compare
note that this PR did not achieve actually performing an abort on closing the dialog, but just as a step in the right direction, just went ahead with merge :) |
This PR that adds an AbortSignal options parameter to "CoreGetFeatures" rpcManager
This was just based on the observation that "SequenceDialog" if a large region is selected and then the dialog is closed, does not cancel the fetch of sequence
This does not yet actually add full aborting for that sequence fetch, because the sequence adapters such as indexedfasta-js et al would have to add abort support to make this really work, but it may be worth it to have the infrastructure added here anyways