This repository was archived by the owner on Mar 26, 2026. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 78
chore: fix CI invocations to use Python 3.9 #1148
Merged
Merged
Changes from all commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
60a9ffc
chore: fix CI invocations to use Python 3.9
software-dov 843c2ba
Multiple fixes and workarounds
software-dov 0d669c9
Authenticate with anonymous credentials
software-dov e8bab9b
Satiate mypy, improve test coverage, fix failing CI
software-dov a2239b1
AutoPep8
software-dov 87a96f0
More satiation
software-dov c3cdc5f
Mypy
software-dov 63983fa
Mypy
software-dov ed5af7e
Merge branch 'main' into showcase-testfix
parthea c83e800
upgrade sphinx to 4.0.1
parthea 0d1144e
chore: fix CI invocations to use Python 3.9
software-dov d5f6c39
Multiple fixes and workarounds
software-dov 3450a6f
Authenticate with anonymous credentials
software-dov b046b34
Satiate mypy, improve test coverage, fix failing CI
software-dov 1f4fd2e
AutoPep8
software-dov 76159c8
More satiation
software-dov 9a12ca0
Mypy
software-dov 527ef04
Mypy
software-dov 924f7be
Merge branch 'showcase-testfix' of https://github.com/software-dov/ga…
software-dov File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1251,7 +1251,7 @@ def test_{{ method.name|snake_case }}_rest_flattened(): | |
| assert len(req.mock_calls) == 1 | ||
| _, args, _ = req.mock_calls[0] | ||
| {% with uri = method.http_options[0].uri %} | ||
| assert path_template.validate("https://%s{{ uri }}" % client.transport._host, args[1]) | ||
| assert path_template.validate("%s{{ uri }}" % client.transport._host, args[1]) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it related to the main goal of this PR? or a bug found independently?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's related to getting showcase tests to pass. We need to pass "http://localhost" as the host, and this mechanism lets the hostname either already have the http:// (preserving the interface for existing users) or not, in which case we can either use https or http. |
||
| {% endwith %} | ||
| {# TODO(kbandes) - reverse-transcode request args to check all request fields #} | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need this? Looks like a heavyweight regexp to do validation. Isn't it safe to assume that if code reaches this poing the url already must be correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not safe to make that assumption. The host is not validated at any point before this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right, but the host itself is not coming as a user input, but is rather generated, so it must be correct and the correctness of that is expected to be tested by relevant gapic-generator tests. Context: I'm just trying to avoid extra logic if possible on every request (anticipating Ads being unhappy with any extra millisecond spend on client layer).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The host may be passed in directly. There is a default, but it is overridable.
This logic happens once during transport construction, so I would be surprised if it shows up during their benchmark tests.