search

googleapis / nodejs-storage

Node.js client for Google Cloud Storage: unified object storage for developers and enterprises, from live data serving to data analytics/ML to data archiving.
https://cloud.google.com/storage/
Apache License 2.0
896 stars 370 forks source link

feat!: Align Support for Storage Emulation #2092

Open danielbankhead opened 1 year ago

danielbankhead commented 1 year ago

Today, baseUrl is equivalent to the STORAGE_EMULATOR_HOST, when available. This leads to limitations when using the client. Here's the relevant code snippet:

https://github.com/googleapis/nodejs-storage/blob/14472691121aa6af0a316c0bf78728edb164c66c/src/storage.ts#L650-L668

Ideally, STORAGE_EMULATOR_HOST should only be a host (without any path associations, e.g. http://localhost:9199) while the client calls the appropriate path on the host (e.g. /storage/v1). That implementation should look something like this: https://github.com/googleapis/nodejs-storage/commit/c75b8b82262dddb794304a71f648bd6e03cafb30

This requires coordination with Firebase and updates to Firebase Storage Emulator. Additionally, we should explore practices to ensure reliability and a consistent experience between Firebase and Google Cloud Storage (e.g. code consolidation and integration testing).

Related:

Background:

mgabeler-lee-6rs commented 1 year ago

The Go GCS client looks like it had some similar issues in the past. There's a fair bit in that repo if you search issues for STORAGE_EMULATOR_HOST, but this ticket is the closest to this and has two PRs linked to fix it: https://github.com/googleapis/google-cloud-go/issues/2476

The changes there look functionally equivalent to what I proposed in #2070, so I'm wondering if the firebase emulator may already have problems with the Go client? I have no experience with the firebase emulator, so I couldn't say for sure.

A thought: as a stepping stone to avoid a "hard break", might it be useful to temporarily permit a second environment variable indicating how to interpret STORAGE_EMULATOR_HOST, with the initial default being the current behavior, and some specific value indicating to use the "new style", and at some point that default changes, and later that second env var is dropped?

mgabeler-lee-6rs commented 1 year ago

Friendly check-in to see if there's any progress on decisions for how this might move forwards?

danielbankhead commented 1 year ago

@mgabeler-lee-6rs hey, thanks for the inquiry; we plan to sync with peer teams on this and other issues over the next few months.

mgabeler-lee-6rs commented 1 year ago

Just checking in on status and confirming that this is still of interest.

danielbankhead commented 1 year ago

@mgabeler-lee-6rs We have alignment between teams and we're likely to resolve this in the current quarter.

mgabeler-lee-6rs commented 1 year ago

Obviously missed Q2, Q3 maybe? :)

danielbankhead commented 1 year ago

@mgabeler-lee-6rs yea, I’m aiming for this half :)

danielbankhead commented 8 months ago

Update: we had a few other high priority features to take care of; however we still have this as a key item to get to shortly.