Fix Storage Control endpoint resolution for TPC#847
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #847 +/- ##
==========================================
+ Coverage 88.19% 88.50% +0.31%
==========================================
Files 15 15
Lines 2989 2993 +4
==========================================
+ Hits 2636 2649 +13
+ Misses 353 344 -9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Mahalaxmibejugam
force-pushed
the
tpc-testing
branch
from
d4740e5 to
8ff2312
Compare
Mahalaxmibejugam
force-pushed
the
tpc-testing
branch
from
8ff2312 to
2962304
Compare
In TPC universe, we must provide the region/location for creating the buckets unlike standard universe which has a default location Added this env variable to use it while initializing the FileSystem
Mahalaxmibejugam
force-pushed
the
tpc-testing
branch
2 times, most recently
from
f81f609 to
f5b4a58
Compare
| } | ||
| if self._location: | ||
| endpoint = self._location.split("://")[-1] | ||
| channel_kwargs["host"] = endpoint |
There was a problem hiding this comment.
in case of self_location being empty, channel is not getting initialised, is that expected ?
There was a problem hiding this comment.
If self._location is empty, channel is initialised in the else block without passing the endpoint. In such cases default storage endpoint would be used in the control client.
Additionally, self._location() will always have a default standard endpoint if it is not specified in the environment variables or during FileSystem creation.
| if self._location and self._location.startswith("http://"): | ||
| host = channel_kwargs["host"] | ||
| channel = grpc.aio.insecure_channel( | ||
| host, options=channel_kwargs.get("options") |
There was a problem hiding this comment.
quota project in insecure grpc channels is intentionally skipped ?
There was a problem hiding this comment.
grpc.aio.insecure_channel() function does not have support for concept of Google-specific options like quota_project_id
We can pass quota_project_id in transport_cls() but passing it along with channel would completely ignore the quota_project_id. Since insecure channels are only used for local emulators(starting with http://) that lack any quota or billing systems, the emulator would completely ignore these GCP billing headers even if they were sent.
| from gcsfs.core import _location | ||
|
|
||
| host = _location() | ||
| return host.startswith("https://") |
There was a problem hiding this comment.
Can custom endpoint for real GCS start with http ?
There was a problem hiding this comment.
I am checking with the TPC team regarding this. If http:// endpoints are supported, we will need to update both the production code and the test utilities. I will include the necessary changes in my next PR if http is confirmed to be supported.
3 participants
This PR fixes the endpoint resolution for the Storage Control API in ExtendedGcsFileSystem to support TPC environments correctly.
Changes