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

[API] Adding dicom upload #6161

Closed
wants to merge 180 commits into from
Closed

Conversation

xlecours
Copy link
Contributor

@xlecours xlecours commented Mar 16, 2020

Copy of #5016

This adds
POST /candidates/<candid>/<visit>/dicoms to upload a dicom.
GET /candidates/<candid>/<visit>/dicoms/<filename>/processes to get the server processes related to that dicom upload (mri_upload)
POST /candidates/<candid>/<visit>/dicoms/<filename>/processes to start a mri_upload server process on that dicom.
GET /candidates/<candid>/<visit>/dicoms/<filename>/processes/<processid> to get the state of a process

This work is based on #4244
To see this PR changes go to xlecours/Loris@api-module-candidates-and-subs...xlecours:adding_dicom_upload

TODO:

kongtiaowang and others added 30 commits October 30, 2017 12:00
[instrument_manager] Modularize instrument manager (aces#3185)
put back useProject config check in candidate select
@xlecours
Copy link
Contributor Author

I am waiting for #6671 to get merged so I can add tests

@driusan driusan added the Blocked PR awaiting the merge, resolution or rejection of an other Pull Request label Aug 11, 2020
@driusan
Copy link
Collaborator

driusan commented Sep 1, 2020

This needs to go in a 0.0.4-dev, not 0.0.3

@ridz1208 ridz1208 removed the Blocked PR awaiting the merge, resolution or rejection of an other Pull Request label Sep 1, 2020
@cmadjar
Copy link
Collaborator

cmadjar commented Jun 8, 2021

@xlecours could you rebase your branch?

@ridz1208
Copy link
Collaborator

@xlecours I rebased this for you here https://github.com/ridz1208/Loris/tree/adding_dicom_upload

if you want I can force push to your branch or you can simply pull it and force push it yourself.

@ridz1208
Copy link
Collaborator

@cmadjar @driusan this one is now rebased and seems to be passing tests

@cmadjar cmadjar assigned driusan and cmadjar and unassigned xlecours Jul 13, 2021
@xlecours xlecours self-assigned this Jul 13, 2021
@xlecours
Copy link
Contributor Author

This is missing integration tests. @xlecours

@driusan
Copy link
Collaborator

driusan commented Aug 10, 2021

@cmadjar can you review?

@xlecours
Copy link
Contributor Author

@xlecours If you do the rebase, @cmadjar agreed to review it again. Also, now that we have a testsuite for the api module, you should add some tests.

@driusan
Copy link
Collaborator

driusan commented Nov 30, 2022

@xlecours are you referring to yourself in the third person? Who's supposed to be writing tests?

@xlecours
Copy link
Contributor Author

@xlecours are you referring to yourself in the third person? Who's supposed to be writing tests?

Yes, I am.

@ridz1208 ridz1208 added the Priority: High PR or issue should be prioritised over others for review and testing label Jul 2, 2024
@driusan
Copy link
Collaborator

driusan commented Nov 7, 2024

Replaced by #9154

@driusan driusan closed this Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API PR or issue introducing/requiring modifications to the API code Feature PR or issue introducing/requiring at least one new feature Needs Rebase PR contains conflicts with the current target branch or was issued to the wrong branch Priority: High PR or issue should be prioritised over others for review and testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants