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

Include support for neuroglancer url for umembargoed zarr and nifti #2063

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

aaronkanzer
Copy link
Member

@aaronkanzer aaronkanzer commented Oct 30, 2024

As per @satra's suggestion for consolidation of LINC --> DANDI, here is some consolidation of providing a Neuroglancer EXTERNAL_SERVICE link for public Nifti and Zarr assets at this time.

Cc @kabilar

@yarikoptic @satra @waxlamp -- if you are curious, here is some documentation for how on the LINC side we went about rendering private assets (e.g. in DANDI case, embargoed essentially): https://github.com/lincbrain/linc-archive/blob/0b1fe19cfdb7075b1c5a5d5a47fec702db948fd7/doc/design/linc_permissions.md#cloudfront-distribution-with-origin-access-identity-control-for-relevant-s3-buckets

I'm hesitant to fully deploy the LINC CloudFront strategy to DANDI until further conversations happen with the Neuroglancer folks. The key blocker here with the LINC neuroglancer<>CloudFront strategy is that it requires 1. another fork of neuroglancer to maintain, and 2. will probably break once data is sourced from MIT infra -- thus conversations need to be had with core neuroglancer regarding injection of creds for private assets (not just those that live on Google Cloud Platform, which seems to be the only cloud vendor natively supported)

Happy to expound further here need be -- thanks all

@aaronkanzer aaronkanzer marked this pull request as ready for review October 30, 2024 15:44
@waxlamp
Copy link
Member

waxlamp commented Oct 31, 2024

@aaronkanzer, I've started looking at this. I need to grok better what the problem is (and your solution); once I do, I think I have a better general solution for situations like this. Stay tuned...

@aaronkanzer
Copy link
Member Author

aaronkanzer commented Oct 31, 2024

@aaronkanzer, I've started looking at this. I need to grok better what the problem is (and your solution); once I do, I think I have a better general solution for situations like this. Stay tuned...

I assume you are alluding to the problem of rendering private zarr in generalneuroglancer at scale? If so, sounds good -- if you'd like a walkthrough of how we set up management of cookies with CloudFront and neuroglancer, happy to do so

Cc @kabilar

@aaronkanzer
Copy link
Member Author

aaronkanzer commented Oct 31, 2024

@waxlamp et. al

For further context, we had this thread with the Google folks prior: google/neuroglancer#507

I wasn't crazy about a solution that exposes the AWS Keys in plaintext in the URL, or having to set up Cognito (which I'm not certain would've effectively solved our problem either), thus the CloudFront solution seemed sensible -- not to mention, we got the performance boost of CloudFront as a CDN (we have frequent Neuroglancer LINC users in Europe), and haven't seen any major cost constraints

name: 'Neuroglancer',
regex: /\.nii(\.gz)?$|\.zarr$/,
maxsize: Infinity,
endpoint: '', // defaults to redirectNeuroglancerUrl logic
Copy link
Member

Choose a reason for hiding this comment

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

I think we should aim for adjusting the "schema" here to reflect this new need -- that we need

Suggested change
endpoint: '', // defaults to redirectNeuroglancerUrl logic
item_handler: 'redirectNeuroglancerUrl',

and then use that in the service code?

@@ -230,7 +230,8 @@
<v-list-item
v-for="el in item.services"
:key="el.name"
:href="el.url"
@click="el.isPublicNeuroglancer ? redirectNeuroglancerUrl(item) : null"
:href="!el.isPublicNeuroglancer ? el.url : null"
Copy link
Member

Choose a reason for hiding this comment

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

as for embargoed -- we are providing URLs unconditionally ATM but they would not even work for embargoed. Filed more generic:

@waxlamp waxlamp marked this pull request as draft December 16, 2024 16:31
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