abstract-sdk icon indicating copy to clipboard operation
abstract-sdk copied to clipboard

Assets endpoints are missing CLI transport

Open tommoor opened this issue 5 years ago • 7 comments

This will soon become a blocker for partial sync. Currently asset download on web / desktop work quite differently – perhaps this is an opportunity to bring them more inline?

Related https://github.com/goabstract/abstract-sdk/issues/84

tommoor avatar Jan 27 '20 23:01 tommoor

Ping on this one.

tommoor avatar Feb 18 '20 18:02 tommoor

The quickest path to get something live here is to handle this in the same way files are currently handled - the API transport would expose the ability to work with the resulting ArrayBuffer, whereas the CLI transport would not. Beyond this type of an asymmetric approach that uses existing CLI functionality, we'd have to work out streaming mechanics in CLI land to match API behavior, which would definitely slow this down.

Is the API consolidation a firm requirement? If not, we can get this live pretty quickly.

bitpshr avatar Feb 18 '20 19:02 bitpshr

I think the API consolidation might be a requirement, otherwise we'd have to do that in the UI codebase – the desktop app would have to know if the assets were coming from the API or CLI and act accordingly.

tommoor avatar Feb 19 '20 23:02 tommoor

Couldn't the desktop application call assets.file({ ... }, { transportMode: ['cli']}), which would mimic the same native CLI functionality today, e.g. saving a file to a location on disk? Is partial sync introducing new application behavior that requires the CLI to actually stream back asset responses so they can be manipulated in some way?

I think I'm lacking context: since web uses API asset download and desktop uses CLI asset download today and things work fine, why do we need to consolidate the underlying semantics of how the API vs. CLI work to move these to SDK requests, especially if application behavior isn't changing?

bitpshr avatar Feb 20 '20 15:02 bitpshr

Partial sync means that the assets will not always be on disk, especially for the usecase of a developer attempting to download assets from a designers branch – they are unlikely to have the project cloned/synced locally – in this case we'll need to fall back to the API transport in desktop.

tommoor avatar Feb 20 '20 18:02 tommoor

Sorry to push the point, but couldn't desktop clients use the already-existing API asset endpoint in the case described above in which an asset is not local? E.g. where a desktop client currently calls into the CLI directly to save a local asset, it could instead call sdkClient.assets.file({ ... }, { transportMode: ['cli', 'api']}), first trying the CLI for the local file, then falling back to the API if the CLI fails. Does some other part of the new partial sync flow require access to the streamed response? If not, we should be OK just exposing what we already have, letting the API stream a response back while the CLI does not.

I agree that we should consolidate the semantics of the API and CLI at some point, but I still don't see how this is a blocker or why we shouldn't at least expose current CLI asset behavior to unblock partial sync / the scenario described above.

bitpshr avatar Feb 21 '20 12:02 bitpshr

Does some other part of the new partial sync flow require access to the streamed response? If not, we should be OK just exposing what we already have, letting the API stream a response back while the CLI does not.

No I don't think so – as long as we can use both transports to save the resulting asset to a specific location on disk with the same interface that's all that should be needed.

tommoor avatar Feb 24 '20 19:02 tommoor