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.

Sign up for GitHub

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

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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