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

Configure dummy pipeline to emit warnings/errors #984

Merged
merged 8 commits into from
Feb 12, 2024

Conversation

corneliusroemer
Copy link
Contributor

@corneliusroemer corneliusroemer commented Feb 11, 2024

This PR configures the dummy preprocessing pipeline to emit errors and warnings when deployed on the server.

E2E is not touched to simplify testing. But the ability to have submissions with errors/warnings is essential to demonstrate/test how our frontend handles them.

For example, I noticed that the new submission review page shows the "release" paperplane icon even for sequences with errors. This wasn't easy to test before the code changes here.

I also add a --dry-run feature to ./deploy.py to make it easiert to see what commands it executes. This helped me understand the precise configuration used by E2E testing.

Preview: https://dummy-warn-error.loculus.org

Resolves #983

Screenshots showing that errors now occur:

image image

@corneliusroemer corneliusroemer added the preview Triggers a deployment to argocd label Feb 11, 2024
@corneliusroemer corneliusroemer marked this pull request as ready for review February 11, 2024 18:30
deploy.py Outdated Show resolved Hide resolved
@theosanderson
Copy link
Member

I'm going to leave approval in case @fengelniederhammer has any thoughts since he's been the driving force behind this dummy pipeline.

preprocessing/dummy/main.py Outdated Show resolved Hide resolved
@corneliusroemer corneliusroemer added preprocessing Issues related to the preprocessing component python config Configuration related issues, i.e. helm processing labels Feb 12, 2024
@corneliusroemer corneliusroemer force-pushed the dummy-warn-error branch 2 times, most recently from 6beff4a to a116223 Compare February 12, 2024 14:00
@corneliusroemer
Copy link
Contributor Author

@fengelniederhammer I've addressed your feedback - the errors/warnings are now deterministic (always on) by default but can be made random with a --randomWarnError flag. Also, I simplified the deployment template.

Copy link
Contributor

@fengelniederhammer fengelniederhammer left a comment

Choose a reason for hiding this comment

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

Thanks, looks good 👍

@corneliusroemer corneliusroemer merged commit e53ead0 into main Feb 12, 2024
12 checks passed
@corneliusroemer corneliusroemer deleted the dummy-warn-error branch February 12, 2024 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
config Configuration related issues, i.e. helm processing preprocessing Issues related to the preprocessing component preview Triggers a deployment to argocd
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Configure test organism to produce predictable warnings to allow testing
3 participants