Make pipeline version per Organism

[fhennig]

resolves #1728

preview URL: https://pipeline-v-per-organism.loculus.org

Summary

The current_processing_pipeline table now has a new column: organism. We now store the current pipeline for each organism.

TODO: How does this affect users? Cornelius said there was previously a constraint to have to use the same version in the values.yaml for all organisms, does that still need to be removed?

Implementation notes

Before this change, we stored "V1" in this table (current_processing_pipeline) - that was simple and could be done without knowing the user configuration. Some of the additional complexity in this PR arises from having to configure a version for the CurrentPipelineVersion for each organism (and also i.e. having to reset this in the tests).

Future work could either include making entries in the table optional, and only filling it as soon as the first sequence for an organism is submitted, or maybe introducing a mechanism to register pipeline versions via API. I opted against making any of these changes to keep the PR small and not change too much of how things are currently functioning.

Testing

• Exisiting tests were updated
• A backend test was added to check that if one pipeline version updates for one organism, another organisms pipeline version is not changed.
• A backend test was added to check that if the initialization runs again, we don't insert v1 rows for organisms that are already in the table.

Docs

Document pipeline version concept better. Relevant link: https://github.com/loculus-project/loculus/blob/main/preprocessing/specification.md#reprocessing

Screenshot

backend change - n/a

PR Checklist

• All necessary documentation has been adapted.
• The implemented feature is covered by an appropriate test.

[fengelniederhammer]

Wow, the pipeline version in the backend is not per organism? But in the values.yml it's per organism, isn't it?

[fhennig]

What do you mean? The pipeline version is internal, so it doesn't appear in the values.yaml if I'm not mistaken.

[chaoran-chen]

The pipeline version also occurs in the Values.yaml because it's also used to deploy the pipelines.

[fengelniederhammer]

loculus/kubernetes/loculus/values.yaml

Lines 1148 to 1150 in 9aa28ab

	preprocessing:
	- &preprocessing
	version: 1

This is the pipeline version, isn't it?

[theosanderson]

Currently for the backend (/in the database) there is a single "latest pipeline version". So if you make a new pipeline version for one organism in the values yaml you have to do so for all organisms. This issue is (intended to be) about addressing that.

[fhennig]

Note to self: The table now starts out empty, somehow it needs to be filled. (I don't think there is a way around the table starting empty now? Because we don't know which organisms there are)

[fhennig]

Update: I found a way to initialize the table on backend-startup.

Now I marked some places in the schema that still need to be adjusted, that currently lead to: "ERROR: more than one row returned by a subquery used as an expression"

[fhennig]

From all the places I had to touch, I think the general handling of this table isn't the best unfortunately. It is different to all the other tables because it starts out with data, and all the others don't.

[fhennig]

Ok at least some of the test failures are because the flyway property I added cannot be injected. maybe I can create a dummy bean for it in the test, or I can somehow make the dependency optional.

Ideally I'd solve the problem of "run x after flyway is finished running" in a different way maybe? Maybe the PostConstruct shouldn't be in the SubmissionDatabaseService, but somewhere else.

Also for anyone who is following along, I wanted to do it this way as a quick way to get it to run, and then iterate on it to make it prettier/less invasive, just so y'all know.

[fhennig]

I've moved the PR out of Draft, because I'm done with the code+tests. I'll look at the docs today.

[fhennig]

I have added some docs stuff:

Not sure if the new subsection is a good idea or not. But I added a page to explain a bit more what a preprocessing pipeline even is, taking some of the text from the specification.

I imagine that in the near future it'd be good to have a couple of sections like "setup", "preprocessing pipeline", "user management", "" sort of like chapters in a manual.

[chaoran-chen]

Thank you very much, Felix! The code looks good to me and the tests are reassuring!

As it is a rather complicated change and it is not easy to test manually, I think that it would be great to have another review by someone. @corneliusroemer, @theosanderson, would you like to take a look?

[corneliusroemer]

I'll review this by Thursday EOD

[corneliusroemer]

A simple/cheap way to test is to use different values for different pipelines in our default values.yaml

There are lines like this per organism (right now version is shared, we can now undo this constraint)

  preprocessing:
    - &preprocessing
      version: 1
      image: ghcr.io/loculus-project/preprocessing-nextclade

Update: I've now done this in dfdc7d3

[fhennig]

That's not quite true, it changes what's allowed in the values.yaml: processing version no longer has to be identical across all organisms.

Ah yes, good point! I meant that it isn't breaking (but I know that we use 'breaking' a little differently). From what I gathered, this constraint isn't enforced anywhere though, or is it? AFAIKT the Helm chart just passes the version through.

[chaoran-chen]

Thanks for fixing!

[fhennig]

I want to test this on staging before merging.