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

fix(metadata): exceptions trying to use the metadata providers #902

Merged
merged 2 commits into from
Nov 21, 2023

Conversation

wayfarer3130
Copy link
Collaborator

@wayfarer3130 wayfarer3130 commented Nov 20, 2023

Context

A recent change started making use of non-string parameters for metadata query, causing some providers to fail. Instead, use varargs for the providers needing more than one query argument.

Changes & Results

Use varargs.

Testing

Run OHIF linked to CS3d, plus the examples htj2kStackBasic and htj2kVolumeBasic. They should still run progressively.

Checklist

PR

  • [] My Pull Request title is descriptive, accurate and follows the
    semantic-release format and guidelines.

Code

Public Documentation Updates

  • [] The documentation page has been updated as necessary for any public API
    additions or removals.

Tested Environment

  • [] "OS:
  • [] "Node version:
  • [] "Browser:

@wayfarer3130 wayfarer3130 requested a review from sedghi November 20, 2023 22:54
Copy link

netlify bot commented Nov 20, 2023

Deploy Preview for cornerstone-3d-docs ready!

Name Link
🔨 Latest commit 7400a50
🔍 Latest deploy log https://app.netlify.com/sites/cornerstone-3d-docs/deploys/655d013ceddc8f00086c6e57
😎 Deploy Preview https://deploy-preview-902--cornerstone-3d-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Member

@sedghi sedghi left a comment

Choose a reason for hiding this comment

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

I think this is once again a side effect of the HTJ2K metadata provider changes. We should modify it to match the signature of others. You can extract the third argument 'stack' from the provider in a different way. I believe we have inadvertently affected several other custom providers in people's apps with this change.

@wayfarer3130 wayfarer3130 requested a review from sedghi November 21, 2023 19:14
Copy link
Member

@sedghi sedghi left a comment

Choose a reason for hiding this comment

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

Thanks makes a lot more sense

@sedghi sedghi changed the title Fix some exceptions trying to use the metadata providers fix(metadata): exceptions trying to use the metadata providers Nov 21, 2023
@sedghi sedghi merged commit a4f1b63 into main Nov 21, 2023
@wayfarer3130 wayfarer3130 deleted the fix/exception-on-metadata-provider branch January 22, 2025 19:04
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.

2 participants