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

Crash on warn? #5023

Open
dnil opened this issue Nov 13, 2024 · 5 comments
Open

Crash on warn? #5023

dnil opened this issue Nov 13, 2024 · 5 comments

Comments

@dnil
Copy link
Collaborator

dnil commented Nov 13, 2024

Apparently the automation sends log output to the equivalent of /dev/null. This is not good procedure. If this is to continue, we should proabably browse through and change a lot of dire warnings to crashes. If no-one reads them, things can go south rather quickly. Crashing early would increase pipeline compatibility and bug finding, no doubt. But be a pain for a while.

@dnil
Copy link
Collaborator Author

dnil commented Nov 14, 2024

A strict mode? Postpone until separating out a more slowly moving parser part?

@dnil
Copy link
Collaborator Author

dnil commented Nov 20, 2024

Let's follow up on https://github.com/Clinical-Genomics/Deviations/issues/715 and changes to the procedure. If the log is read at least once in a while on prod, and on verification testing, I think we should be fine. It does simplify day to day life if we don't crash on everything. Compare #5048 which would have been a crash as well. 🤷

@AnnaLeinfelt
Copy link

If the log is read at least once in a while on prod, and on verification testing, I think we should be fine.

What is prod here, is it the bioinfo production team? What frequency would be needed?

@northwestwitch
Copy link
Member

northwestwitch commented Jan 15, 2025

What is prod here, is it the bioinfo production team? What frequency would be needed?

yes, the bioinfo production team. If t's warnings, I'm not sure about the frequency. Especially if we don't define some things they should be looking for and report if they show up. it's a quite abstract thing that won't happen I'm afraid. On the other hand if we introduce a crash in the code instead of a warning that would definitely get attention, but would interrupt production until the thing is fixed. Did you have anything in mind @dnil?

@dnil
Copy link
Collaborator Author

dnil commented Jan 15, 2025

We will still try to keep a balance in Scout it so we fail on critical issues, but failing on all db inconsistency like missing genes, changed annotations, duplicated variants etc is going to be quite a bother on production bioinformatics for some time.
As long as we actually read the logs during pipeline validation/verification, and dito for Scout and Scout database updates we should be somewhat ok, or what do you think @northwestwitch?

But cg needs an option to show logs for sure. We had another one of those issues during nf-core/raredisease dev a few days back (https://github.com/Clinical-Genomics/MTP-RAREDISEASE/issues/35) that would have been so much smoother if they had the log output.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants