Skip to content
This repository was archived by the owner on Feb 23, 2026. It is now read-only.

feat: add grpc transcoding + tests - #259

Merged
vam-google merged 20 commits into
googleapis:mainfrom
yihjenku:transcode-takeover
Sep 15, 2021
Merged

feat: add grpc transcoding + tests#259
vam-google merged 20 commits into
googleapis:mainfrom
yihjenku:transcode-takeover

Conversation

@yihjenku

@yihjenkuyihjenku commented Aug 25, 2021

Copy link
Copy Markdown
Contributor

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #<issue_number_goes_here> 🦕

@yihjenku
yihjenku requested a review from a teamAugust 25, 2021 17:19
@google-clagoogle-claBot added the cla: yes This human has signed the Contributor License Agreement. label Aug 25, 2021
@tseavertseaver added kokoro:force-run Add this label to force Kokoro to re-run the tests. owlbot:run Add this label to trigger the Owlbot post processor. labels Aug 25, 2021
@yoshi-kokoroyoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 25, 2021
@gcf-owl-botgcf-owl-botBot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Aug 25, 2021

@tseavertseaver left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yihjenku Thank you very much for the patch! I've tagged the Owlbot to run here, which should fix any linting issues (black wants spaces between key and value in dict comprehensions, for instance). The testcases look well-thought out.

Comment threadgoogle/api_core/path_template.py Outdated
@partheaparthea added the owlbot:run Add this label to trigger the Owlbot post processor. label Aug 25, 2021
@gcf-owl-botgcf-owl-botBot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Aug 25, 2021
Comment threadgoogle/api_core/path_template.py Outdated
Comment threadtests/unit/test_path_template.py
@tseavertseaver added kokoro:force-run Add this label to force Kokoro to re-run the tests. owlbot:run Add this label to trigger the Owlbot post processor. labels Aug 25, 2021
@gcf-owl-botgcf-owl-botBot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Aug 25, 2021
@yoshi-kokoroyoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 25, 2021
Comment threadgoogle/api_core/path_template.py
@tseavertseaver added kokoro:force-run Add this label to force Kokoro to re-run the tests. owlbot:run Add this label to trigger the Owlbot post processor. labels Aug 25, 2021
@yoshi-kokoroyoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 25, 2021
@gcf-owl-botgcf-owl-botBot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Aug 25, 2021
@tseaver

Copy link
Copy Markdown
Contributor

Coverage failure flake reported in #260.

@tseavertseaver added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 25, 2021
@yoshi-kokoroyoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 25, 2021
Comment threadgoogle/api_core/path_template.py
Comment threadgoogle/api_core/path_template.py Outdated
Comment threadgoogle/api_core/path_template.py Outdated
Comment threadgoogle/api_core/path_template.py Outdated
Comment threadgoogle/api_core/path_template.py

@vam-googlevam-google left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but please keep in mind that we may have to circle back to optimize the implementation if it does not meet Ads performance requirements.

Comment threadgoogle/api_core/path_template.py
@vam-google
vam-google requested review from a teamSeptember 10, 2021 23:15
Comment threadgoogle/api_core/path_template.py
Comment threadgoogle/api_core/path_template.py Outdated
Comment threadgoogle/api_core/path_template.py Outdated
Comment threadgoogle/api_core/path_template.py
@vam-google
vam-google merged commit afe0fa1 into googleapis:mainSep 15, 2021
This was referenced May 30, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla: yesThis human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants

@yihjenku@tseaver@jameslynnwu@kbandes@alexander-fenster@busunkim96@vam-google@parthea@yoshi-kokoro@yon-mg