-
Notifications
You must be signed in to change notification settings - Fork 9
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
Implement stage-free delivery for project deliveries. #48
base: master
Are you sure you want to change the base?
Implement stage-free delivery for project deliveries. #48
Conversation
This is because they are required by DDS when creating a project.
It seems this method confuses pytest too much and makes mocking impossible.
OBS: there is a lot of code repetition in there and it's probably possible to make these tests more concise by writting some helper functions in the base class, but these tests are soon going to be obsolete and are going to be removed once the new delivery endpoint is released and the old ones are decommissioned.
This is in preparation for the big clean-up
This set a project as completed in the database
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! I left some comments for you.
Regarding skip_delivery
I agree that it can be removed. When doing full integration testing / validation we want to deliver data to DDS. It is not ideal that we upload to the "production env", but since there is no "Staging" version of DDS, I don't really see how we would do it differently.
Co-authored-by: Matilda Åslin <[email protected]>
This reverts commit e7a735b. Turns out I didn't need this function in the end.
Nice comments! Ready for the second round of review now. Then I think it's ready for merging and testing in staging after we update snpseq_packs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! ✔️
What problems does this PR solve?
The delivery system has been switched to DDS. This program provides an option to upload files to a specific location. This makes the staging step unnecessary. Skipping this step would both reduce delivery times and simplify the code
The delivery service supports three delivery modes: project, runfolder, and runfolder for projects. This PR introduces a new delivery route for projects (the simplest case). The goal is to test if this works as expected and to gather feedback before we apply this change to the other (more complex) delivery modes. All former delivery routes remain available.
Some design questions to think about:
As it is now, I have not implemented the
skip_delivery
option, is this something we want to keep?Risk analysis:
How has the changes been tested?
In addition to automatic tests, has any manual testing been carried out? Unit tests and integration tests have been updated.
Reasons for careful code review
If any of the boxes below are checked, extra careful code review should be initiated.