Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

[Brainbrowser] file_url query param #8335

Merged
merged 3 commits into from
Jan 23, 2024
Merged

[Brainbrowser] file_url query param #8335

merged 3 commits into from
Jan 23, 2024

Conversation

laemtl
Copy link
Contributor

@laemtl laemtl commented Jan 23, 2023 •
edited
Loading

Based on #8643

Support for an additional query parameter to pass file urls:

brainbrowser/?minc_id=32,33&file_url=https://loris.ca/api/v0.0.3/candidates/867710/V1/images/inhance_867710_V1_F18FEOBVOSEM_001.mnc,https://loris.ca/api/v0.0.3/candidates/867710/V1/images/inhance_867710_V1_F18FEOBVOSEM_002.mnc

This is useful from some particular projects where files are stored on an external server (no files db).
minc_id and file_url can be both supplied with values and the result will be merged in the response.

@laemtl laemtl requested a review from xlecours January 23, 2023 17:23
@laemtl laemtl force-pushed the brainbrowser-files-query-param-and-404 branch 2 times, most recently from a6408ed to 42be75b Compare January 23, 2023 17:51
xlecours
xlecours approved these changes Jan 23, 2023
View reviewed changes
Copy link
Contributor

@xlecours xlecours left a comment

Choose a reason for hiding this comment

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

I have to test it now :)

modules/brainbrowser/js/brainbrowser.loris.js Outdated Show resolved Hide resolved
@laemtl laemtl changed the title [Brainbrowser] Add support for filePath query param [Brainbrowser] Add support for fileurl query param Jan 23, 2023
@laemtl laemtl changed the title [Brainbrowser] Add support for fileurl query param [Brainbrowser] Add support for file_url query param Jan 23, 2023
@laemtl laemtl force-pushed the brainbrowser-files-query-param-and-404 branch 2 times, most recently from 4baa979 to 9b19a57 Compare January 23, 2023 18:08
@laemtl laemtl added the Security PR patches a vulnerability, makes resource access changes, or updates dependencies label Feb 21, 2023
@laemtl laemtl closed this Mar 16, 2023
@laemtl laemtl reopened this Apr 5, 2023
@laemtl laemtl added this to the 25.0.0 milestone Apr 5, 2023
@laemtl
Copy link
Contributor Author

laemtl commented Apr 5, 2023 •
edited
Loading

@xlecours @driusan Reviewing this I believe the fix to display errors should be part of the release since it may fix all the reported issues for the modules: #8016, #7681. We can remove the changes that covers the support for an additional query parameter to pass file urls if this is an issue.

@laemtl laemtl mentioned this pull request Apr 5, 2023
Closed
16 tasks
@laemtl laemtl changed the title [Brainbrowser] Add support for file_url query param [Brainbrowser] Display error and add support for file_url query param Apr 5, 2023
@laemtl laemtl changed the title [Brainbrowser] Display error and add support for file_url query param [Brainbrowser] Display error and file_url query param Apr 5, 2023
@laemtl laemtl changed the base branch from main to 25.0-release April 5, 2023 16:21
@xlecours xlecours added the Critical to release PR or issue is key for the release to which it has been assigned label Apr 11, 2023
@laemtl laemtl assigned laemtl and unassigned driusan and xlecours Apr 18, 2023
@laemtl laemtl removed the Critical to release PR or issue is key for the release to which it has been assigned label Apr 19, 2023
@laemtl laemtl changed the title [Brainbrowser] Display error and file_url query param [Brainbrowser] file_url query param Apr 19, 2023
@laemtl laemtl removed this from the 25.0.0 milestone Apr 19, 2023
@driusan driusan changed the base branch from 25.0-release to main May 11, 2023 14:35
@driusan
Copy link
Collaborator

driusan commented Jan 11, 2024

@laemtl This can't be merged because of conflicts

@laemtl laemtl force-pushed the brainbrowser-files-query-param-and-404 branch from ece73ee to c7ffdaa Compare January 16, 2024 18:17
@laemtl
Copy link
Contributor Author

laemtl commented Jan 16, 2024

@driusan Ready

@laemtl laemtl removed their assignment Jan 16, 2024
@driusan driusan merged commit 18d1453 into aces:main Jan 23, 2024
19 checks passed
@ridz1208 ridz1208 added this to the 26.0.0 milestone Apr 9, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@xlecours xlecours xlecours approved these changes

Assignees

@driusan driusan

Labels
Security PR patches a vulnerability, makes resource access changes, or updates dependencies
Projects
None yet
Milestone
26.0.0
Development

Successfully merging this pull request may close these issues.
4 participants
@laemtl @driusan @xlecours @ridz1208