use substitutions for external service endpoints

[magland]

In light of this issue on neurosift
flatironinstitute/neurosift#112

it is necessary to use the DANDI API asset download URL rather than the s3 bucket URL for the endpoint of the neurosift external service.

For example, here's an s3 bucket URL
https://dandiarchive.s3.amazonaws.com/blobs/d64/0c3/d640c378-7025-4c3b-a334-169ec3df4f3f

which corresponds to the following DANDI API download URL:
https://api.dandiarchive.org/api/assets/34a21ebe-c756-4da4-a30b-f1a838a6430b/download/

As described in the above quoted issue: In the cases of embargoed dandisets, it is necessary for neurosift to receive the API download URL so that it can use proper authentication to retrieve the signed download URL. The direct s3 bucket URL will never work.

This PR enables this, while still utilizing the s3 URL for the other services.

A detail about the modified code: While the s3 bucket url is directly available on the path.asset object, the api download url is not. So I needed to construct the url, and that required knowing whether or not it's the staging site.

In short, this PR is needed to allow Neurosift to be able to view assets from embargoed dandisets.

[yarikoptic]

That's GREAT! Thank you @magland ! I left some comments/recommendations.

[yarikoptic]

Those are both URLs to the asset download. So I think we could/should make them a little more uniform, what about

Suggested change

	$s3_url$: trimEnd(path.asset.url, '/'),
	$api_download_url$: `${baseApiUrl}/api/assets/${path.asset.asset_id}/download/`,
	$asset_s3_download_url$: trimEnd(path.asset.url, '/'),
	$asset_api_download_url$: `${baseApiUrl}/api/assets/${path.asset.asset_id}/download/`,

so in the future we might expand the listing here without fear of collision?

[yarikoptic]

BTW , in #1655 (does this PR supersedes it?) we seems also wanted to expose asset_id. I think it would be safe to include it here as well but I guess we could add more as demand comes.

[magland]

Thanks @yarikoptic !

I have adjusted the strings as you suggested and added a $asset_id$ (which of course is unused at this point)

Yes, I think this is accomplishing the idea laid out in #1655

[magland]

BTW, do I have the correct way of testing whether it's the staging site?

const isStaging = process.env.VUE_APP_SENTRY_ENVIRONMENT === 'staging';

[magland]

Thanks @yarikoptic I have made your suggested changes.

[magland]

@yarikoptic @waxlamp I'm not sure if this is feasible, but I was hoping this PR could be merged and released sometime this week, because I am giving a presentation on the 10th, and it would be really nice to have this in place. Happy to make adjustments if needed.

[waxlamp]

@magland I'm going to try something today that may solve the problem. Stand by.

[waxlamp]

@magland, please check out #1692. I believe that PR (inspired by this one) is a simpler way to solve the problem you are facing.

I would prefer to defer this "API" creation in the external services integrations to a real design round so that we don't hastily get stuck with suboptimal choices. That is to say, I'd like to solve your problem in as direct as a mode as possible while addressing the ideas of #1655 with more deliberation (which I invite you to collaborate on if you're interested).

Let me know what you think.

[magland]

Thanks @waxlamp . I think that will do the trick!

From the perspective of neurosift, I believe it would be better to get the same type of URL from dandi-archive regardless of whether or not it is embargoed. But if #1692 could be released soon, then that would satisfy my short-term need.

[yarikoptic]

I would prefer to defer this "API" creation in the external services integrations to a real design round so that we don't hastily get stuck with suboptimal choices.

note that in this PR we are changing internal supplementary code so we would be able to as easily fix it up to some more optimal solution happen we need it -- it is not an external registry ATM etc. But I think it is moving in the right direction and would allow for better basis for any possible design round if it ever to happen.

[satra]

@waxlamp - actually one of the things i wanted in the other PR is the dandiset specific asset download url, not the general one. that would allow in essence for someone to fetch relevant dandiset info or for externallinks such as videos, the video url. also since this is clicked not from a global search but a view within the dandiset, i think it's appropriate to have the other url.

[waxlamp]

the other PR

Do you mean #1655 or #1692?

dandiset specific asset download url, not the general one

When you say "dandiset specific", do you mean the longer URL that is "nested" under a /dandisets/{dandiset_id}/... URL?

[satra]

correct - sorry was in meetings. yes, i would like that asset url to change to the one that is nested under /dandisets/