Skip to content
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

Detect assembly loading error and encapsulate error instead of failing at app level #2236

Merged
merged 10 commits into from
Aug 23, 2021

Conversation

cmdcolin
Copy link
Collaborator

@cmdcolin cmdcolin commented Aug 19, 2021

This PR allows detecting that an assembly failed to load for either the case of

(a) a defaultSession loads a view who has a displayedRegion assembly that fails e.g. 404
(b) browsing the import form and selecting an assembly gives an error e.g. 404

See #2235

The volvox demo data has a "volvox404" assembly added that is always a 404. To test, you can also generate other errors by turning off network in chrome or inserting a throw into the sequence adapter's getRegions function or similar.

This results in a couple places where we have to be careful to clear the error state. There is the view model's error state, and there is the import form components error state. The import form components error state is initialized with the model's error state, and import form onSubmit clears the model's error state

This is also for the LGV only, and if we want to put it into other parts of the app, we may need to perform more effort, or create some better abstractions.

Screenshot from 2021-08-19 18-02-17
screenshot showing the 404 from volvox

@github-actions github-actions bot added the needs label triage Needs a label to show in changelog (breaking, enhancement, bug, documentation, or internal) label Aug 19, 2021
@cmdcolin cmdcolin marked this pull request as draft August 19, 2021 22:13
@cmdcolin cmdcolin force-pushed the assembly_loading_error branch from 5a457f2 to 569c645 Compare August 19, 2021 22:15
@cmdcolin cmdcolin added enhancement New feature or request and removed needs label triage Needs a label to show in changelog (breaking, enhancement, bug, documentation, or internal) labels Aug 19, 2021
@cmdcolin
Copy link
Collaborator Author

Target branch is #2188 but probably doesn't necessarily have to be

@cmdcolin cmdcolin force-pushed the assembly_loading_error branch from 569c645 to 83a613d Compare August 19, 2021 22:22
@codecov
Copy link

codecov bot commented Aug 19, 2021

Codecov Report

Merging #2236 (7bf1a16) into main (2c74146) will increase coverage by 0.03%.
The diff coverage is 71.75%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2236      +/-   ##
==========================================
+ Coverage   62.98%   63.01%   +0.03%     
==========================================
  Files         485      488       +3     
  Lines       22716    22760      +44     
  Branches     5159     5170      +11     
==========================================
+ Hits        14308    14343      +35     
- Misses       8138     8147       +9     
  Partials      270      270              
Impacted Files Coverage Δ
...r-view/src/CircularView/components/CircularView.js 94.44% <ø> (+3.53%) ⬆️
...ew/src/LinearSyntenyView/components/ImportForm.tsx 0.00% <0.00%> (ø)
...nce/src/IndexedFastaAdapter/IndexedFastaAdapter.ts 81.25% <0.00%> (+5.57%) ⬆️
packages/core/assemblyManager/assemblyManager.ts 59.21% <42.85%> (-0.79%) ⬇️
packages/core/assemblyManager/assembly.ts 86.66% <57.14%> (ø)
...iew/src/LinearGenomeView/components/ImportForm.tsx 60.86% <60.97%> (-0.33%) ⬇️
plugins/dotplot-view/src/DotplotView/model.ts 65.26% <63.74%> (+2.76%) ⬆️
...ar-view/src/CircularView/components/ImportForm.tsx 73.07% <73.07%> (ø)
...lot-view/src/DotplotView/components/ImportForm.tsx 89.28% <82.35%> (+5.28%) ⬆️
...s/dotplot-view/src/DotplotView/components/Axes.tsx 86.53% <86.53%> (ø)
... and 15 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2c74146...7bf1a16. Read the comment docs.

@cmdcolin cmdcolin changed the title Detect assembly loading error in LGV and encapsulate the error instead of failing at app level Detect assembly loading error and encapsulate error instead of failing at app level Aug 20, 2021
@cmdcolin cmdcolin force-pushed the assembly_loading_error branch 4 times, most recently from 83e437e to c6b50d6 Compare August 20, 2021 19:38
@cmdcolin cmdcolin marked this pull request as ready for review August 20, 2021 19:54
@cmdcolin cmdcolin force-pushed the assembly_selector branch 4 times, most recently from b8b9241 to 45f63e2 Compare August 21, 2021 06:02
@cmdcolin cmdcolin force-pushed the assembly_loading_error branch from c6b50d6 to 7bf1a16 Compare August 21, 2021 13:38
@cmdcolin cmdcolin changed the base branch from assembly_selector to main August 21, 2021 13:38
@rbuels rbuels merged commit 1b4c565 into main Aug 23, 2021
@rbuels rbuels deleted the assembly_loading_error branch August 23, 2021 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants