feat(odsp-client): lookupTemporaryBlobUrl functionality #25815
Conversation
github-actions
bot
added
dependencies
| try { | ||
| if ( | ||
| runtimeInternal !== undefined && | ||
| typeof (runtimeInternal as { lookupTemporaryBlobStorageId?: unknown }) |
There was a problem hiding this comment.
Shouldn't need this guard since odsp-client brings along a modern version of the runtime (via fluid-static) and therefore knows it's going to have a version with this function.
That said, would be nice to add a more explicit check and throw a more helpful error if it's missing in runtime-utils rather than a plain cast.
| * @returns The blob URL if found and the blob is not pending, undefined otherwise | ||
| */ | ||
| function lookupOdspBlobURL( | ||
| runtimeInternal: IContainerRuntimeInternal, |
There was a problem hiding this comment.
The runtime-utils lookupTemporaryBobStorageId takes an IContainerRuntime, so this should just be an IContainerRuntime.
| public constructor( | ||
| container: IContainer, | ||
| private readonly odspResolvedUrl?: IOdspResolvedUrl, | ||
| private readonly containerRuntimeInternal?: IContainerRuntimeInternal, |
There was a problem hiding this comment.
Here also prefer to pass around an IContainerRuntime. In general my preference is to defer casting to the internal type until just when it's needed.
| const resolvedUrl = container.resolvedUrl; | ||
| const odspResolvedUrl = | ||
| resolvedUrl && isOdspResolvedUrl(resolvedUrl) ? resolvedUrl : undefined; | ||
| return new OdspContainerServices(container, odspResolvedUrl, runtimeInternal); |
There was a problem hiding this comment.
Here you have everything you need to create a partially-applied function lookupTemporaryBlobURL(handle) and hand that function directly to the OdspContainerServices rather than making OdspContainerServices construct it itself. This will let you avoid passing large objects around.
| } | ||
|
|
||
| /** | ||
| * Unclear if this is the best way to go about accessing the runtime internal from the entry point. |
There was a problem hiding this comment.
Instead I think I'd expect to plumb a lookupTemporaryBlobStorageId onto IRootDataObject, in general IRootDataObject is the chokepoint for exposing stuff in the runtime to the outside world.
2 participants
Description
Portion of #25597. Adds
lookupTemporaryBlobUrlfunctionality to OdspContainerServices. This new method allows consumers to see a handle's current blob url with no guarantees of the URL's longevity.Reviewer Guidance
The way I'm accessing ContainerRuntimeInternal is technically correct, but doesn't feel great. Should I alter some internal types used by
fluid-static?