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

corrected _getSharedMediaTypes to enable methods like retrieveStudy w… #56

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Christian-e-S
Copy link
Contributor

…hich check for commonMediaType === MEDIATYPES.DICOM

Problem description:
Some functions like "retrieveStudy(options)" check for commonMediaType === MEDIATYPES.DICOM and throw "Media type ${commonMediaType} is not supported for retrieval of study." when the condition is not met.

Current implementation of _getSharedMediaTypes returns "application/" for "application/dicom" which will not meet the condition above.

With the modification that I have introduced, "application/dicom" will stay intact.

Please note: There was something similar in previous versions of this code.

…hich check for commonMediaType === MEDIATYPES.DICOM
@pieper
Copy link
Contributor

pieper commented Jun 21, 2024

@Christian-e-S is this still valid? I'm not familiar with the issue myself.

@lam0620
Copy link

lam0620 commented Jul 9, 2024

I pass below param to retrieveStudy(options)

{
        studyInstanceUID,
        mediaTypes: [{
          transferSyntaxUID: '1.2.840.10008.1.2.4.90',
          mediaType: 'image/jp2'
        }
        ],
 }

I got an error:
Media type image/ is not supported for retrieval of study.

@pieper
Copy link
Contributor

pieper commented Jul 9, 2024

Okay, this sounds reasonable, but I'm wary to change this code since it's very hard to test for any side effects.

Can you create an issue and reference it here that describes the scenario in which this happens? I.e. what types of data is involved (SM? US? MR?) and what servers?

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

Successfully merging this pull request may close these issues.

3 participants