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
888 stars 368 forks source link

fix: Improve STORAGE_EMULATOR_HOST usage #2470

Closed le0pard closed 1 month ago

le0pard commented 1 month ago

Problem

Expected behaviour

Fix

google-cla[bot] commented 1 month ago

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

conventional-commit-lint-gcf[bot] commented 1 month ago

🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use automerge label. Good luck human!

-- conventional-commit-lint bot https://conventionalcommits.org/

ddelgrosso1 commented 1 month ago

Hi @le0pard we appreciate the contribution. However, there is a reason we do not append pathing to STORAGE_EMULATOR_HOST. For more details see: https://github.com/googleapis/nodejs-storage/pull/2087 and https://github.com/googleapis/nodejs-storage/issues/2092

le0pard commented 1 month ago

Thanks @ddelgrosso1 Should we just introduce new env variable, which will work the same as apiEndpoint, while team busy make STORAGE_EMULATOR_HOST work with Firebase Storage emulator ?

ddelgrosso1 commented 1 month ago

@le0pard fair question. Let me check internally with the firebase team as to what the current status is. The problem with adding a new variable is it is one more thing to maintain and deprecating something else becomes a long task.

Is there a problem with using apiEndpoint for your use case?

le0pard commented 1 month ago

Thanks @ddelgrosso1

apiEndpoint is exactly how I resolve this issue for now. Problem, that in project we are using node.js and python code. In python sdk STORAGE_EMULATOR_HOST works as expected ( https://github.com/googleapis/python-storage/blob/main/google/cloud/storage/client.py#L168-L171 ) , but not in node.js sdk. So in python code I dont need create any additional code for sdk to switch endpoint for local development. Just pass env variable in docker container.

In JS codebase I need create separate env variable like GCLOUD_API_ENDPOINT and write code, which will check this env variable to make sdk work in same way, as python:

const {
  Storage: GoogleStorage
} = require('@google-cloud/storage')

const STORAGE_ENDPOINT = process.env.GCLOUD_API_ENDPOINT ?? undefined

const storage = new GoogleStorage({
  apiEndpoint: STORAGE_ENDPOINT
})

And of course I cannot share same STORAGE_EMULATOR_HOST env variable, because it is "break" node.js sdk right now