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

Add Apollo assembly and feature importing status to JBrowse 2 jobs widget #289

Merged
merged 9 commits into from
Oct 26, 2023

Conversation

carolinebridge
Copy link
Contributor

@carolinebridge carolinebridge commented Sep 28, 2023

Resolves #234

Dependent on JBrowse core changes: GMOD/jbrowse-components#3951 (merged), changes will be in v2.7.0

summary of changes

  • Adds a new ApolloJobModel to update the jobs widget UI where appropriate
  • Adds a safeguard to the add assembly UI to prevent a user from uploading an assembly with an existing name (an operation that will always fail and takes some time if the file is large)
  • Utilizes the new jobs manager to update the widget UI when a user attempts to upload a new assembly or upload new features for an assembly

…ssembly name in assembly upload ;; utilizes job manager in the steps in adding an assembly and features to that assembly
@carolinebridge carolinebridge self-assigned this Sep 28, 2023
@carolinebridge carolinebridge added the enhancement New feature or request label Sep 28, 2023
@carolinebridge carolinebridge marked this pull request as ready for review October 3, 2023 15:45
@carolinebridge
Copy link
Contributor Author

carolinebridge commented Oct 3, 2023

👇 tests are failing because WidgetType 'JobsListWidget' not found, which is included in the jbrowse-components PR

Copy link
Contributor

@garrettjstevens garrettjstevens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only works if a single job is running at a time, which we can't guarantee. For example, I started an upload of a larger assembly file, then while it was running uploaded a small file which finished basically immediately. Then, when the larger one tried to update its percentage, the app crashed since the newer job had cleared out the name, etc. in the Apollo jobs manager. Some jobs also might not be uploads, too, so they may be even more likely to run at the same time.

I think the jobs manager needs to either 1) keep track of each submitted job or 2) be stateless, so instead of storing jobs it's a set of utility functions for helping Changes, etc. add info directly to the job status widget. I lean toward option 2.

Also, the abort controller should be tied to the underlying fetch. So for example, in AddAssembly.tsx, there should be a const controller = new AbortController() and then later in the code

const { signal } = controller
const response = await apolloFetchFile(url, {
  method: 'POST',
  body: formData,
  signal,
})

Then when submitting the job, you'll have

jobStatusWidget.addJob({
  name,
  statusMessage,
  progressPct,
  cancelCallback: () => {
    controller.abort()
  },
})

That will be the same controller as was created in AddAssembly.tsx. This way, when someone clicks on the cancel button in the jobs widget, it will abort the fetch.

packages/jbrowse-plugin-apollo/src/ApolloJobModel.ts Outdated Show resolved Hide resolved
packages/jbrowse-plugin-apollo/src/ApolloJobModel.ts Outdated Show resolved Hide resolved
packages/jbrowse-plugin-apollo/src/ApolloJobModel.ts Outdated Show resolved Hide resolved
@garrettjstevens
Copy link
Contributor

After merging main, it looks like the tests aren't passing because the "Features are being added" message that "importFeatures" is looking for does not exist after this change. I tried to update the test, but didn't have any luck. @dariober, would you be able to take a look at this? Instead of checking for the previous message, I think you can now tell features have been added when the "Completed jobs" section has two entries that both say "All operations successful".

image

@garrettjstevens
Copy link
Contributor

@dariober never mind, I was able to figure out the tests :)

@garrettjstevens garrettjstevens merged commit c9fecf2 into main Oct 26, 2023
@garrettjstevens garrettjstevens deleted the feature-import-status branch October 26, 2023 22:58
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.

Add Apollo assembly and feature importing status to JBrowse 2 jobs widget
2 participants