Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Fix rest transport logic #1039

Merged
merged 2 commits into from Oct 25, 2021
Merged

Conversation

vam-google
Copy link
Contributor

This includes

  1. Do not include asyncio tests in the generated tests, because rest transport does not have asynio client.
  2. Generate body field mock values for generated tests (otherwise grpc transcodding logic would fail).
  3. Make always_use_jwt_access=True default for rest clients (grpc already does that) to match expected calls in generated tests.
  4. Fix mypy errors for AuthorizedSession by ignoring it
  5. Include operations_v1 conditionally, only if the client has lro

There are few more fixes left, which are expected to be fixed in separate PRs:

  1. message->to_dict->message roundrtip problem for int64 types is expected to be fixed by fix: setting 64bit fields from strings supported proto-plus-python#267
  2. builtins conflicts (license_ vs license as field name) is expected to be fixed by a TBD PR

This includes
1) Do not include asyncio tests in the generated tests, because rest transport does not have asynio client.
2) Generate body field mock values for generated tests (otherwise grpc transcodding logic would fail).
3) Make `always_use_jwt_access=True` default for rest clients (grpc already does that) to match expected calls in generated tests.
4) Fix mypy errors for `AuthorizedSession` by ignoring it
5) Include operations_v1 conditionally, only if the client has lro

There are few more fixes left, which are expected to be fixed in separate PRs.

1) `message->to_dict->message` roundrtip problem for int64 types is expected to be fixed by googleapis/proto-plus-python#267
2) builtins conflicts (`license_` vs `license` as field name) is expected to be fixed by a TBD PR
@vam-google vam-google requested a review from a team as a code owner October 22, 2021 00:16
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Oct 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants