fix(main): ensure compatibility with the latest connect-python
#123
Conversation
BREAKING CHANGE: Upgrade from connect-python 0.4.2 to 0.6.0 The connect-python library underwent a major API change between versions 0.4.x and 0.6.0, requiring updates throughout the codebase. Changes: - Update dependencies: split connect-python[compiler] into separate packages - connect-python>=0.6.0,<0.7 - protoc-gen-connect-python>=0.6.0,<0.7 - Fix binary name: protoc-gen-connect_python → protoc-gen-connect-python The new version uses hyphens instead of underscores in the binary name - Regenerate proto files with new API - Deleted 15 old *_pb2_connect.py files - Generated 15 new *_connect.py files - Fixed imports to use relative paths - Update SDK implementation (kas_connect_rpc_client.py) - Replace urllib3.PoolManager with httpx.Client - Use AccessServiceClientSync instead of AccessServiceClient - Update client instantiation: base_url and session parameters - Change extra_headers parameter to headers - Add proper context manager usage for HTTP clients - Update test mocks (test_kas_client.py) - Patch httpx.Client instead of urllib3.PoolManager - Update mock setup for context managers - Fix import paths for new module structure - Update proto module exports (kas/__init__.py) - Export AccessServiceClientSync instead of AsyncAccessServiceClient
Summary of ChangesHello @b-long, 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 primarily updates the Highlights
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 review |
![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 successfully updates the otdf-python-proto submodule, primarily focusing on upgrading the connect-python dependency and its related tooling. The changes are comprehensive, including updates to the protobuf generation scripts, regeneration of client code, and necessary adaptations in the KAS client and its tests. The migration from urllib3 to httpx is a positive change, improving the HTTP client handling. The code modifications are consistent with the dependency upgrade and appear to be correct. I have no further suggestions.
There was a problem hiding this comment.
Code Review
This pull request updates the otdf-python-proto submodule, which involves upgrading connect-python and regenerating the protobuf client code. The changes correctly adapt the SDK to the new library version, including switching from urllib3 to httpx and updating client instantiation. The tests have also been updated accordingly. My main concern is that while the KAS service's __init__.py is updated to export the new client classes, other services' __init__.py files are not included in this PR. If they exist, they are likely broken by this change, preventing other services from being used. Additionally, there's an opportunity to improve performance by reusing httpx.Client instances across multiple requests.
|
/gemini review |
There was a problem hiding this comment.
Pull request overview
This PR updates the otdf-python-proto submodule to use the newer connect-python version 0.6.x, migrating from version 0.4.x. The update involves regenerating protobuf client code to use the new Connect RPC library API, which changes HTTP client handling from urllib3 to httpx, updates client class names, and modifies the parameter naming convention.
Key Changes:
- Upgraded
connect-pythonfrom 0.4.x to 0.6.x and added separateprotoc-gen-connect-pythondependency - Migrated HTTP client from
urllib3.PoolManagertohttpx.Clientwith lazy initialization and resource management - Updated Connect RPC client class names (e.g.,
AccessServiceClient→AccessServiceClientSync)
Reviewed changes
Copilot reviewed 50 out of 52 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| pyproject.toml | Updated connect-python dependency versions and added protoc-gen-connect-python |
| src/otdf_python/kas_connect_rpc_client.py | Migrated from urllib3 to httpx, added context manager support and lazy client initialization |
| src/otdf_python/kas_client.py | Added context manager methods for proper resource cleanup |
| tests/test_kas_client.py | Updated test mocks to reflect new httpx and AccessServiceClientSync usage |
| otdf-python-proto files | Regenerated protobuf connect files with new connect-python 0.6.x tooling |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Code Review
This pull request updates the otdf-python-proto submodule, primarily to upgrade the connect-python dependency. The changes include regenerating the protobuf client files to align with the new version, which now uses httpx instead of urllib3. The client code in kas_client.py and kas_connect_rpc_client.py has been updated to support the new API, including adding context manager support for better resource management. The tests have also been updated accordingly. I've identified a potential resource leak issue in the backward-compatibility implementation and suggested a fix.
b-long
changed the title
otdf-python-proto submoduleotdf-python-proto submodule
|
/gemini review |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 51 out of 53 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Code Review
This pull request updates the otdf-python-proto submodule, primarily to upgrade the connect-python dependency. The changes are extensive, touching generated protobuf client code, dependency specifications, and client implementation logic to adapt to the new connect-python API. The migration appears to be handled correctly, including updating package names, client instantiation, and method call signatures. A significant improvement is the introduction of context manager support for KASClient and KASConnectRPCClient, which ensures proper resource management of HTTP clients. The tests have also been updated accordingly to reflect these changes. Overall, this is a solid update that modernizes the RPC client implementation. I have one minor suggestion for improving code clarity.
b-long
changed the title
otdf-python-proto submoduleconnect-python
This pull request updates the Connect Python plugin integration and refines the way generated protobuf modules are imported and exposed in the package. The changes ensure compatibility with the latest
connect-pythonnaming conventions, improve dependency instructions, and add the generated Connect client code for theauthorizationservice.Connect Python plugin integration and naming updates:
protoc-gen-connect_pythonto the correctprotoc-gen-connect-pythonnaming inbuf.gen.yamland related scripts, ensuring consistency with the latest plugin naming conventions. [1] [2] [3] [4]connect-pythonandprotoc-gen-connect-pythonfor generating Connect clients.Generated Connect client code and import structure:
authorization_connect.pyfile, providing both async and sync Connect client classes for theAuthorizationService(including service protocols and ASGI/WSGI application classes).authorizationmodule to expose the new Connect client classes via its__init__.py.__init__.pyto import submodules likeauthorization,entityresolution,wellknownconfiguration, andpolicylazily, avoiding import errors from missing generated protobuf files and only directly exposing stable modules in__all__.