Sirius bindings

[mvisani]

Finished binding for Sirius. All the tests pass, the fuzzer has not detected any problems.

We tested with @oolonek an MGF file with both default parameters and specific parameters for the ENPKG pipeline, they both work.

[oolonek]

I don't think we used a docker but the follwoing bash scripts https://github.com/enpkg/enpkg_full?tab=readme-ov-file#-install-sirius-locally. @mvisani just told be he was keen to peek in the process of a Docker creation !!
Normally login are correctly handled at

emi-monorepo/bindings/sirius/src/sirius.rs

Line 48 in 62bf8fb

let sirius_username = env::var("SIRIUS_USERNAME").map_err(|_| {

[LucaCappelletti94]

Ok so, at this time in the pipeline, we are installing Sirius in the Docker (using that script). This means that we can add to the test suite the execution of Sirius as part of the C.I.

Are the Sirius login credentials always needed or only for some tasks? I seemed to recall they were only needed for a subset of them.

[oolonek]

Ok we had misunderstood that you wanted to create a dedicated Sirius docker image (and not just Sirius install within a Docker as in https://github.com/earth-metabolome-initiative/emikg/blob/09ddc2ebd8dba1746ca9a8b71eed89be48b1d39a/enrichers/enricher-dirty-pipeline/Dockerfile#L28C1-L30C1) I understand that for now this latter option is OK ?

Regarding logins I think that they are required for tasks such as zodiac, canopus and similar workflows used by Sirius. And these are the typical tasks we want to run, so I guess we should login each time.

[LucaCappelletti94]

If the login credentials need to be in plain text, we cannot use them in the CI. If we cannot use them in the CI but still want to test Sirius in the CI, we can test all of the things that do not require the login.

At this time we are currently always requiring a user of the bindings to login, but most likely we only want for the user to login when the builder configuration includes some routine that requires the login.

If we refactor that part to only require the login when it is strictly needed, we can (partially) test Sirius in the CI.

[oolonek]

For me this looks good for a first version of the Sirius bindings :)
I'll let @luca validate the merge !

[oolonek]

If the login credentials need to be in plain text, we cannot use them in the CI. If we cannot use them in the CI but still want to test Sirius in the CI, we can test all of the things that do not require the login.

At this time we are currently always requiring a user of the bindings to login, but most likely we only want for the user to login when the builder configuration includes some routine that requires the login.

If we refactor that part to only require the login when it is strictly needed, we can (partially) test Sirius in the CI.

Couldn't we use Repository Secrets for this ? What is the issue of login each time ?

[LucaCappelletti94]

I need to look into the repo secrets, it could be an option. Do you know which of the settings exactly require the login?

[oolonek]

I need to look into the repo secrets, it could be an option. Do you know which of the settings exactly require the login?

Will dig into that. Pasting here for the record sirius-ms/sirius#146

[LucaCappelletti94]

Great job with the documentation, now there is the matter of the README, which should contain the badges for the crate, documentation and GitHub actions, plus an explanation of what the crate is for and a couple of examples showing how to use it (and maybe also an explanation about how to install Sirius itself).

You can use as example the README of HyperLogLog.

[LucaCappelletti94]

What was the issue that caused the pipeline to get sirius?

[mvisani]

It is in the changes of the bindings/sirius/src/sirius.rs from last commit.

We changed from this :

command.args(&args).spawn().expect("Sirius failed to start");

To this:

// Add arguments and spawn the command
let mut child = command.args(&args).spawn().expect("Sirius failed to start");
let status = child.wait().expect("Failed to wait on child");

On my side it still runs super slow but I would be ready to bet that it is because of my computer.

[mvisani]

I've added some examples and errors in the documentation. Tell me if something is missing or needs to be added ! :)