Open qhaas opened 2 years ago
Hi, thanks for filing this. As you noted, python admin SDK support is planned but we can't provide any hard dates as to when it will be available. We can keep this issue open to track any work that's being done but as always, we welcome any and all community contributions. If someone wants to take a stab at this, we will be more than happy to provide guidance and reviews.
Thank you @qhaas for a good minimal reproduction script. I dug in a little bit; I am by no means an expert in this code base, so it took a lot of console.log
s in the emulator and breakpoints in the python code. I found a few issues with how the python client sends out requests. I found patches for uploading and deletion.
The emulator expects a name
query parameter for file uploads. The python client does not provide this; the URL looks like this:
/upload/storage/v1/b/redacted-bucket.appspot.com/o?uploadType=multipart
This accounts for the (first) HTTP 400. Proposed patch here.
Further downstream, the storage emulator breaks apart the multi-part upload by checking the boundary string provided in the header, but it errors out here again ({"code":400,"message":"Unexpected number of parts in request body"}
). The python client provides a content-type header which looks like this:
multipart/related; boundary="===============8101563892775960909=="
The docstring in the emulator strongly suggests it doesn't expect a boundary string with quotes like this. Since a quoted-string is a valid boundary, this seems like a bug in emulator code. Proposed patch here.
Next error: {"code":400,"message":"Failed to parse multipart request body part. Missing content type."}
. The emulator expects to see the string Content-Type
, the Python client is supplying content-type
. The emulator just needs to be case insensitive here. Proposed patch here.
Got a 404:
Traceback (most recent call last):
File "/home/redacted/Code/github.com/jack-michaud/redacted/test_files.py", line 23, in <module>
blobUpload.make_public()
File "/home/redacted/.cache/pypoetry/virtualenvs/redacted/lib/python3.9/site-packages/google/cloud/storage/blob.py", line 3269, in make_public
self.acl.all().grant_read()
File "/home/redacted/.cache/pypoetry/virtualenvs/redacted/lib/python3.9/site-packages/google/cloud/storage/acl.py", line 396, in all
return self.entity("allUsers")
File "/home/redacted/.cache/pypoetry/virtualenvs/redacted/lib/python3.9/site-packages/google/cloud/storage/acl.py", line 351, in entity
if self.has_entity(entity):
File "/home/redacted/.cache/pypoetry/virtualenvs/redacted/lib/python3.9/site-packages/google/cloud/storage/acl.py", line 302, in has_entity
self._ensure_loaded()
File "/home/redacted/.cache/pypoetry/virtualenvs/redacted/lib/python3.9/site-packages/google/cloud/storage/acl.py", line 229, in _ensure_loaded
self.reload(timeout=timeout)
File "/home/redacted/.cache/pypoetry/virtualenvs/redacted/lib/python3.9/site-packages/google/cloud/storage/acl.py", line 462, in reload
found = client._get_resource(
File "/home/redacted/.cache/pypoetry/virtualenvs/redacted/lib/python3.9/site-packages/google/cloud/storage/client.py", line 364, in _get_resource
return self._connection.api_request(
File "/home/redacted/.cache/pypoetry/virtualenvs/redacted/lib/python3.9/site-packages/google/cloud/storage/_http.py", line 80, in api_request
return call()
File "/home/redacted/.cache/pypoetry/virtualenvs/redacted/lib/python3.9/site-packages/google/api_core/retry.py", line 283, in retry_wrapped_func
return retry_target(
File "/home/redacted/.cache/pypoetry/virtualenvs/redacted/lib/python3.9/site-packages/google/api_core/retry.py", line 190, in retry_target
return target()
File "/home/redacted/.cache/pypoetry/virtualenvs/redacted/lib/python3.9/site-packages/google/cloud/_http/__init__.py", line 483, in api_request
raise exceptions.from_http_response(response)
google.api_core.exceptions.NotFound: 404 GET http://localhost:9199/storage/v1/b/redacted-bucket.appspot.com/o/storage_test%2Ftest.txt/acl?prettyPrint=false: No such object: storage/v1/b/redacted-bucket.appspot.com/o/storage_test/test.txt/acl
I think what may be happening here is that there is no GET */acl
route so it's going to the object getter route and it can't find the file storage_test%2Ftest.txt/acl
.
I'm not sure what to do here. The emulator does not support ACL rules, so I don't want to create a route just to make the client happy. The nodejs client avoids this by not GETting the ACL when it needs to update the rule, so this may be a limitation on the python client.
Seems to work okay on my setup.
google.api_core.exceptions.MethodNotImplemented: 501 DELETE http://localhost:9199/storage/v1/b/bucket-redacted.appspot.com/o/storage_test%2Ftest.txt?prettyPrint=false: Not Implemented
There are two options here; change the client to use a URL the emulator can handle (it looks like it can handle /b/:bucketId/o/:objectId/acl
) or change the emulator to handle the path.
It feels like adding an alias to that route is the lowest risk and can potentially fix other similar issues (https://github.com/firebase/firebase-tools/issues/3508#issuecomment-963561373). Proposed patch here.
If these approaches make sense, I can make a PRs in firebase-tools and python-storage to proceed with the fixes.
There are still some outstanding issues to go:
@tonyjhuang I opened a couple PRs to fix these issues https://github.com/googleapis/python-storage/pull/761 https://github.com/firebase/firebase-tools/pull/4423), but I'm currently looking for a thumbs up before unmarking as draft.
I've been using these branches of the Firebase emulator and the python admin SDK for 4 months internally with no issue, so I'm unmarking these PRs as draft.
Hi @jack-michaud I've been working through some python gcs sdk issues on the storage emulator and I merged in some changes to strip quotes (https://github.com/firebase/firebase-tools/pull/4825) and to make content type case insensitive (https://github.com/firebase/firebase-tools/pull/4824). I think it would still be good to merge in these two changes from you (https://github.com/jack-michaud/firebase-tools/commit/c0045d9d89600d620a4746bbf2ac2e541e4cc81d, https://github.com/jack-michaud/firebase-tools/commit/e5df6986df20624f4d89267489b3b53e286fe80e) so if you could update your pr and add some tests I'd love to take a look.
Thank you @Yuangwang! I will add tests to those changes. Also, this may be outside of your purview, but have you taken a look at https://github.com/googleapis/python-storage/pull/761? This fix is required for file uploads from python to work with the storage emulator.
@jack-michaud I also pushed a change recently to have the storage emulator fallback on reading the name from the file metadata (https://github.com/firebase/firebase-tools/pull/4832). I tested it with the python gcs sdks and it should fix the upload issue you mentioned!
Hi @jack-michaud this pr should be a good reference for where the tests should go https://github.com/firebase/firebase-tools/pull/5209/files.
Looking at the commits you linked again I think the additional get route was added in another pr, so the only one to add would be the delete one (https://github.com/jack-michaud/firebase-tools/commit/c0045d9d89600d620a4746bbf2ac2e541e4cc81d)
It looks like our test coverage is actually a bit weak here for delete so we don't have a testcase for the flow yet. To add one you could do something like: upload(testfile) delete(testfile) check that files been deleted.
Feel free to take a stab at it, but we're also happy to create our own pr to get in the new route + test changes since the coverage was missing to begin with. Let me know if you take a stab at it, otherwise we can make the pr in the coming weeks
Thank you for your comment... I deleted my comment when I found that exact PR and saw that there was a test associated with it. So I'm adding the test for Delete! Thank you for your kindness in searching for that on my behalf.
Do we have any update on this?
Same error here:
/functions/venv/lib/python3.11/site-packages/google/cloud/_http/__init__.py", line 494, in api_request
> raise exceptions.from_http_response(response)
> google.api_core.exceptions.NotFound: 404 GET http://127.0.0.1:9199/storage/v1/b/MY-PROJECT.appspot.com?projection=noAcl&prettyPrint=false: No such object: storage/v1/b/MY-PROJECT.appspot.com
I'll pick this back up when I have some time over the next week.
I remember getting a little confused when trying to find an existing test to pull inspiration from.
I'll try to find the existing test code that tests these code path again.
Looking at the commits you linked again I think the additional get route was added in another pr, so the only one to add would be the delete one (https://github.com/jack-michaud/firebase-tools/commit/c0045d9d89600d620a4746bbf2ac2e541e4cc81d)
Update: all new changes in my PR were integrated by a separate author adding support for the V1 API in b0798fb1fe -- so deletion should now work.
From my notes it seems like the only thing the client does not support yet is ACL rules, but the last I checked emulator itself doesn't support ACL rules.
Summary: Allow use of Firebase storage emulators via the
google.cloud.storage
API.Background: As Firebase developers, we often use the Firebase emulator suite during development. Given Firebase is hosted on Google Cloud (GC), many features run on said GC and are compatible with GC APIs. The Firebase Storage Emulator lacks python client and admin bindings, python admin support is planned, but no word on when it will be available. The admin sdk is overkill for our use case since we just want to upload / download files with python.
Apparently, third party GC storage emulators are supported by the GC python-storage API (see #376, #324). Given we already use the python-storage API to interact with the GC hosted Firebase, it seems reasonable it should also support Google's own Firebase Storage emulator like it does said third-party GC Storage emulators. It is possibly related to this Firebase Storage emulator issue.
Example with Google Cloud: Consider this snippet, it works fine with our Google Cloud hosted Firebase project.
The following environment variables were set:
GOOGLE_APPLICATION_CREDENTIALS
: Path to PKI fileGCP_PROJECT
: GC hosted Firebase Project IDfrom google.cloud import storage import os
print("Initializing storage client") if os.getenv("STORAGE_EMULATOR_HOST"): storageClient = storage.Client.create_anonymous_client() storageClient.project = "none" else: storageClient = storage.Client() print("Success")
print("Initializing bucket") bucket = storageClient.bucket(os.environ['GCP_PROJECT']+'.appspot.com') print("Success")
print("Writing storage test entry") blobUpload = bucket.blob('storage_test/test.txt') blobUpload.upload_from_filename('upload_test.txt') blobUpload.make_public() print("Success")
print("Reading storage test entry") blobDownload = bucket.blob('storage_test/test.txt') blobDownload.download_to_filename('download_test.txt') os.remove('download_test.txt') print("Success")
print("Deleting storage test entry") blobDelete = bucket.blob('storage_test/test.txt') blobDelete.delete() print("Success")
Initializing storage client Success Initializing bucket Success Writing storage test entry Success Reading storage test entry Success Deleting storage test entry Success
Traceback (most recent call last): File "/REDACTED/lib64/python3.9/site-packages/google/cloud/storage/blob.py", line 2577, in upload_from_file created_json = self._do_upload( File "/REDACTED/lib64/python3.9/site-packages/google/cloud/storage/blob.py", line 2379, in _do_upload response = self._do_multipart_upload( File "/REDACTED/lib64/python3.9/site-packages/google/cloud/storage/blob.py", line 1914, in _do_multipart_upload response = upload.transmit( File "/REDACTED/lib64/python3.9/site-packages/google/resumable_media/requests/upload.py", line 153, in transmit return _request_helpers.wait_and_retry( File "/REDACTED/lib64/python3.9/site-packages/google/resumable_media/requests/_request_helpers.py", line 147, in wait_and_retry response = func() File "/REDACTED/lib64/python3.9/site-packages/google/resumable_media/requests/upload.py", line 149, in retriable_request self._process_response(result) File "/REDACTED/lib64/python3.9/site-packages/google/resumable_media/_upload.py", line 114, in _process_response _helpers.require_status_code(response, (http.client.OK,), self._get_status_code) File "/REDACTED/lib64/python3.9/site-packages/google/resumable_media/_helpers.py", line 105, in require_status_code raise common.InvalidResponse( google.resumable_media.common.InvalidResponse: ('Request failed with status code', 400, 'Expected one of', <HTTPStatus.OK: 200>)
During handling of the above exception, another exception occurred:
Traceback (most recent call last): File "/REDACTED/test_gc.py", line 20, in
blobUpload.upload_from_filename('upload_test.txt')
File "/REDACTED/lib64/python3.9/site-packages/google/cloud/storage/blob.py", line 2718, in upload_from_filename
self.upload_from_file(
File "/REDACTED/lib64/python3.9/site-packages/google/cloud/storage/blob.py", line 2594, in upload_from_file
_raise_from_invalid_response(exc)
File "/REDACTED/lib64/python3.9/site-packages/google/cloud/storage/blob.py", line 4466, in _raise_from_invalid_response
raise exceptions.from_http_status(response.status_code, message, response=response)
google.api_core.exceptions.BadRequest: 400 POST http://localhost:9199/upload/storage/v1/b/REDACTED.appspot.com/o?uploadType=multipart: Bad Request: ('Request failed with status code', 400, 'Expected one of', <HTTPStatus.OK: 200>)