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

Support custom GET parameters when retrieving images #105

Open
medihack opened this issue Oct 22, 2024 · 3 comments
Open

Support custom GET parameters when retrieving images #105

medihack opened this issue Oct 22, 2024 · 3 comments

Comments

@medihack
Copy link

medihack commented Oct 22, 2024

Would it be possible to support sending additional parameters when retrieving images? _http_get_multipart_application_dicom already has a params parameter to add "Additional HTTP GET query parameters", but it is not used by the retrieve functions that call this method.
Background: We are developing a custom DICOMweb API server (OpenRadX ADIT) that can process additional parameters (e.g. for pseudonymization).
Would you accept a PR for such a feature?

@pieper
Copy link
Member

pieper commented Oct 22, 2024

Would you accept a PR for such a feature?

Sounds reasonable to me, so long as it's backwards compatible and uses basically standard web features.

@CPBridge
Copy link
Collaborator

CPBridge commented Oct 22, 2024

The custom parameters are used to share underlying code between the "standard variants" of retrieving studies/series/images. E.g. adding the /metadata or /rendered parameters to give retrieve-study_metadata and retrieve_instance_frames_rendered etc. But this is not exposed to allow other "non-standard variants" currently. The retrieve_study, retrieve_series, retrieve_instance methods have clearly-defined standard meanings.

In my opinion therefore, it may be better to add new public methods that allow adding custom parameters (like retrieve-study_metadata and retrieve_instance_frames_rendered also do) rather than add parameters to the existing methods as this could be less confusing to the user. These could share much of the implementation under the hood with the existing methods.

@medihack
Copy link
Author

I wonder (even if we have no direct use for it in ADIT right now) if it should be possible to provide optional parameters to all functions (e.g., also for the search functions). But this would lead to much code duplication and a bloated API for an optional, server-specific parameter. As a user, I would prefer to have an optional params parameter added to each method. That said, I am okay with both options and would implement what you decide.

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

No branches or pull requests

3 participants