Skip to content

[ENH] Allow override of webapi-url from URL parameter - #238

Open
bpinsard wants to merge 3 commits into
nipreps:mainfrom
bpinsard:enh/online_webapi_url_load
Open

[ENH] Allow override of webapi-url from URL parameter#238
bpinsard wants to merge 3 commits into
nipreps:mainfrom
bpinsard:enh/online_webapi_url_load

Conversation

@bpinsard

Copy link
Copy Markdown

Simple attempt at implementing nipreps/mriqc#1422 .

@effigies
effigies force-pushed the enh/online_webapi_url_load branch from 42cf0e6 to ea37074 Compare March 3, 2026 17:56
@codecov

codecov Bot commented Mar 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.75%. Comparing base (2faab66) to head (42f09c1).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #238      +/-   ##
==========================================
+ Coverage   66.45%   66.75%   +0.29%     
==========================================
  Files          27       27              
  Lines        3023     3023              
  Branches      401      401              
==========================================
+ Hits         2009     2018       +9     
+ Misses        897      892       -5     
+ Partials      117      113       -4     
Flag Coverage Δ
unittests 62.65% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@effigies
effigies force-pushed the enh/online_webapi_url_load branch from 8cb96ae to 3db1cdc Compare March 10, 2026 15:02
Comment thread nireports/assembler/data/rating-widget/footer.tpl Outdated
@effigies

Copy link
Copy Markdown
Member

Why do we pass the URL to Authorization header? Is that specific to MRIQC API?

I don't know anything about this. @oesteban (or maybe @rwblair?) could probably answer better.

@bpinsard
bpinsard force-pushed the enh/online_webapi_url_load branch from 3b532d6 to 42f09c1 Compare March 10, 2026 17:20
@bpinsard

Copy link
Copy Markdown
Author

Why do we pass the URL to Authorization header? Is that specific to MRIQC API?

I don't know anything about this. @oesteban (or maybe @rwblair?) could probably answer better.

Nevermind, I guess a rebase messed-up things, weird that the github diff was showing that as the previous line in history.

@effigies effigies left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This falls back to current behavior, so LGTM as something to be going on with. Will merge after a day if no further discussion.

@effigies effigies changed the title [ENH] allow override of webapi-url from URL parameter [ENH] Allow override of webapi-url from URL parameter Mar 10, 2026
@effigies effigies self-assigned this Mar 10, 2026
@oesteban

Copy link
Copy Markdown
Member

Why cannot it be done at generation time using metadata.endpoint without this override?

@bpinsard

Copy link
Copy Markdown
Author

It is more for the case where API endpoint is unknown at generation, or might depend on which platform the generated reports get reviewed on.

We do not have an implemented usecase now, but I am looking into being able to open reports and have the rating widget point to some webhooks to save the results in a datalad dataset. As this is in a distributed multi-site setting, this URL might change depending on the environment.

@oesteban

Copy link
Copy Markdown
Member

This would also require passing a token (or unset it).

I'm wondering, why not grep + sed twice (one sed for the url, one for the token)?

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