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

When using ?input-fasta= url query param without specifying dataset, web auto-starts analysis (prematurely) #1436

Open
corneliusroemer opened this issue Apr 17, 2024 · 5 comments
Labels
package: nextclade_web t:bug Type: bug, error, something isn't working

Comments

@corneliusroemer
Copy link
Member

corneliusroemer commented Apr 17, 2024

When you just pass the input-fasta URL param, Nextclade web autostarts the analysis, but it shouldn't.

It should instead just load the sequences and do auto-dataset-detection

Repro:

  1. Load https://clades.nextstrain.org/?dataset-url=gh:alex-ranieri/denvLineages@main@/Nextclade_V3_data/DENV1 to set up the storage/cookies
  2. Load https://clades.nextstrain.org/?input-fasta=https://raw.githubusercontent.com/inrb-labgenpath/Mpox_sequencing_Kamituga/main/submission01_mpox47_2024.fasta

Instead the dataset that's set in browser storage/cookies is used (wrongly)

It's possible that in order to reproduce it, it's necessary to first load a custom dataset (so that the storage is set appropriately

@corneliusroemer corneliusroemer added t:bug Type: bug, error, something isn't working good first issue Good for newcomers help wanted Extra attention is needed needs triage Mark for review and label assignment package: nextclade_web and removed good first issue Good for newcomers help wanted Extra attention is needed labels Apr 17, 2024
@corneliusroemer
Copy link
Member Author

Per Ivan this works as intended. So what I'm suggesting is to change behavior to the following:

  • when someone passes URL params, don’t use state
  • only use state when people are not using URL params

@ivan-aksamentov
Copy link
Member

Note for myself: "not using state", selected dataset in this case, implies dataset autodetection at boot and then running with it. Not entirely straightforward with the current code organization. Need to move things around probabaly.

@ivan-aksamentov ivan-aksamentov removed the needs triage Mark for review and label assignment label Apr 17, 2024
@corneliusroemer
Copy link
Member Author

Ah autodetect is a state that should be kept, states that shouldn't be used are all the ones that can be URL param configured, assume they are unset for a reason.

I'm not sure autodetect behavior is very crucial here one way or another.

@ivan-aksamentov
Copy link
Member

ivan-aksamentov commented Apr 17, 2024

I mean the situation if you decide we don't use dataset saved in local storage, and when it's not passed as URL param, but input-fasta is passed. Then what dataset would you use?

Or you propose to not start unless both input-fasta and dataset-name is passed?

There are so many possible combinations that I am confused now.

@corneliusroemer
Copy link
Member Author

I mean the situation if you decide we don't use dataset saved in local storage, and when it's not passed as URL param, but input-fasta is passed. Then what dataset would you use?

Don't assume one, I'd say? Only use autorun when people specify a dataset to be safe. Yeah this might technically be breaking some people's links but I don't think many people use just input fasta and no specified dataset. We could check logs just in case :)

Or you propose to not start unless both input-fasta and dataset-name is passed?

Indeed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: nextclade_web t:bug Type: bug, error, something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants