chore: librarian onboard pull request: 20260407T145439Z - #16574
Conversation
Sorry, something went wrong.
Sorry, something went wrong.
There was a problem hiding this comment.
Code Review
This pull request introduces the google-cloud-appoptimize library, a new Python client for the App Optimize API (v1beta). The changes encompass the full suite of generated client code, including sync and async interfaces, gRPC and REST transports, and associated samples and tests. Feedback highlights critical bugs in the REST transport's debug logging and example interceptor code. Suggestions for improvement include optimizing query parameter processing using MessageToDict and ensuring deterministic output by programmatically sorting dictionary keys in path parsing and query parameter generation.
Sorry, something went wrong.
| ): # pragma: NO COVER | ||
| try: | ||
| response_payload = app_optimize.Report.to_json(response) | ||
| except: |
There was a problem hiding this comment.
The logging code incorrectly attempts to call to_json on the requests.Response object (response) instead of the parsed proto message (resp). This will cause an error during logging when DEBUG level is enabled.
| except: | |
| response_payload = app_optimize.Report.to_json(resp) |
Sorry, something went wrong.
| response | ||
| ) | ||
| except: |
There was a problem hiding this comment.
The logging code incorrectly attempts to call to_json on the requests.Response object (response) instead of the parsed proto message (resp). This will cause an error during logging when DEBUG level is enabled.
response_payload = app_optimize.ListReportsResponse.to_json(
resp
)
Sorry, something went wrong.
| ): # pragma: NO COVER | ||
| try: | ||
| response_payload = app_optimize.ReadReportResponse.to_json(response) | ||
| except: |
There was a problem hiding this comment.
The logging code incorrectly attempts to call to_json on the requests.Response object (response) instead of the parsed proto message (resp). This will cause an error during logging when DEBUG level is enabled.
| except: | |
| response_payload = app_optimize.ReadReportResponse.to_json(resp) |
Sorry, something went wrong.
| json_format.MessageToJson( | ||
| transcoded_request["query_params"], | ||
| use_integers_for_enums=True, | ||
| ) | ||
| ) | ||
| query_params.update( |
There was a problem hiding this comment.
Using json_format.MessageToDict is more efficient than serializing to a JSON string and then parsing it back into a dictionary. This improvement can be applied to all similar occurrences in this file.
| json_format.MessageToJson( | |
| transcoded_request["query_params"], | |
| use_integers_for_enums=True, | |
| ) | |
| ) | |
| query_params.update( | |
| query_params = json_format.MessageToDict( | |
| transcoded_request["query_params"], | |
| use_integers_for_enums=True, | |
| ) |
Sorry, something went wrong.
|
|
||
| query_params["$alt"] = "json;enum-encoding=int" | ||
| return query_params | ||
|
|
There was a problem hiding this comment.
To ensure dictionary keys remain sorted without manual effort, programmatically sort the dictionary before returning it instead of relying on manual ordering in the code. This ensures deterministic behavior for the generated query parameters. This improvement can be applied to all _get_query_params_json methods in this file.
| return dict(sorted(query_params.items())) |
- To ensure dictionary keys remain sorted without manual effort, programmatically sort the dictionary before returning it (e.g., using dict(sorted(metadata.items()))) instead of relying on manual ordering in the code.
Sorry, something went wrong.
| class MyCustomAppOptimizeInterceptor(AppOptimizeRestInterceptor): | ||
| def pre_create_report(self, request, metadata): | ||
| logging.log(f"Received request: {request}") | ||
| return request, metadata |
There was a problem hiding this comment.
The example interceptor uses logging.log without specifying a log level, which will raise a TypeError. It should use a specific level method like logging.info or provide the level as the first argument. This applies to all logging calls in the example.
| return request, metadata | |
| logging.info(f"Received request: {request}") |
Sorry, something went wrong.
| path, | ||
| ) | ||
| return m.groupdict() if m else {} | ||
|
|
There was a problem hiding this comment.
To ensure dictionary keys remain sorted without manual effort, programmatically sort the dictionary before returning it instead of relying on manual ordering in the code. This applies to all path parsing methods in this class.
| return dict(sorted(m.groupdict().items())) if m else {} |
- To ensure dictionary keys remain sorted without manual effort, programmatically sort the dictionary before returning it (e.g., using dict(sorted(metadata.items()))) instead of relying on manual ordering in the code.
Sorry, something went wrong.
chalmerlowe
left a comment
There was a problem hiding this comment.
LGTM
Sorry, something went wrong.
PR created by the Librarian CLI to onboard a new Cloud Client Library.
BEGIN_COMMIT
feat: onboard a new library
PiperOrigin-RevId: 895558469
Library-IDs: google-cloud-appoptimize
END_COMMIT
Librarian Version: v0.8.3
Language Image: us-central1-docker.pkg.dev/cloud-sdk-librarian-prod/images-prod/python-librarian-generator@sha256:234b9d1f2ddb057ed7ac6a38db0bf8163d839c65c6cf88ade52530cddebce59e