feat(storage): refactor the async code to accomodate JSON and GRPC implementation.#1747
feat(storage): refactor the async code to accomodate JSON and GRPC implementation.#1747googlyrahman wants to merge 4 commits intogoogleapis:asyncfrom
Conversation
product-auto-label
bot
added
size: l
googlyrahman
changed the title
Summary of ChangesHello @googlyrahman, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant architectural refactoring to the asynchronous client and connection layers. The primary goal is to establish a flexible foundation that can seamlessly accommodate both JSON-based HTTP and gRPC communication protocols. By abstracting the connection logic and specializing the JSON implementation, the system is now better positioned for future expansion and protocol diversity, enhancing modularity and maintainability. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request is a solid architectural refactoring that separates connection logic from the async client, preparing for both JSON and gRPC support. My review has identified a critical issue in the new AsyncClient.close() method, which violates a repository rule regarding resource cleanup and would cause it to fail. Additionally, there are a couple of medium-severity suggestions for code cleanup and improving test coverage.
google/cloud/storage/_experimental/asyncio/utility/async_json_connection.py
Show resolved
Hide resolved
googlyrahman
force-pushed
the
feb8
branch
2 times, most recently
from
ed21dee to
0e9a775
Compare
product-auto-label
bot
added
size: xl
googlyrahman
force-pushed
the
feb8
branch
2 times, most recently
from
5c6297f to
b8d1a13
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
The pull request refactors the asynchronous storage client to support both JSON and gRPC protocols by introducing an abstract AsyncConnection base class and a protocol-specific AsyncJSONConnection. Overall, the refactoring improves modularity and aligns the async implementation with the library's multi-protocol goals. I have identified a few issues regarding consistency with the existing synchronous implementation, specifically around user agent string construction and a minor typo in a copyright header.
| if self._client_info.user_agent is None: | ||
| self._client_info.user_agent = AGENT_VERSION | ||
| else: | ||
| self._client_info.user_agent = ( | ||
| f"{self._client_info.user_agent} {AGENT_VERSION}" | ||
| ) |
There was a problem hiding this comment.
The user agent string construction should be kept consistent with the existing JSON client implementation in google/cloud/storage/_http.py. The current implementation is missing the leading and trailing spaces around the AGENT_VERSION string, which are present in the synchronous implementation to ensure proper formatting when concatenated with other identifiers.
if self._client_info.user_agent is None:
self._client_info.user_agent = ""
if AGENT_VERSION not in self._client_info.user_agent:
self._client_info.user_agent += f" {AGENT_VERSION} "References
- The user agent string construction, including extra spaces, should be kept consistent with the existing JSON client implementation.
googlyrahman
added
kokoro:run
yoshi-kokoro
removed
kokoro:run
googlyrahman
added
kokoro:run
yoshi-kokoro
removed
kokoro:run
chore: remove python 3.9 support. Details in b/483015736
3 participants
Removed private methods from AsyncClient and BaseClient: To support using the same asynchronous client for both HTTP and gRPC, we removed several private methods. These methods required HTTP-specific arguments (such as method) that are not applicable to gRPC interactions, making them unnecessary for the unified async client implementation.
Refactored
AsyncConnection: Split theAsyncConnection(was HTTP based) class into anAsyncConnection(Abstract Base Class) and anAsyncJSONConnection(Implementation) to allow for more flexible protocol support.