feat: Universe domain support for discovery based libraries. #746
Conversation
This file contains 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
jskeet
approved these changes
Feb 22, 2024
| .WithBlockBody(resourceProperties.Zip(Resources).Select(pair => pair.First.Assign(New(ctx.Type(pair.Second.Typ))(This))).ToArray()) | ||
| .WithBlockBody( | ||
| resourceProperties.Zip(Resources).Select(pair => pair.First.Assign(New(ctx.Type(pair.Second.Typ))(This))) | ||
| .Append(baseUriProperty.Assign(Call("GetEffectiveUri")(IdentifierName("BaseUriOverride"), baseUriValue))) |
There was a problem hiding this comment.
Nit: indent these three lines more? (So they're indented relative to resourceProperties.Zip?)
amanda-tarafa
added a commit
to googleapis/google-api-dotnet-client
that referenced
this pull request
Feb 22, 2024
- The universe domain may be specified through the client service initializer. - A helper method to calculate effective URIs that takes into account the universe domain. This method is to be used by generated code, see googleapis/gapic-generator-csharp#746. - The universe domain is passed as a request option for the credential to validate against when intercepting and before setting the token.
amanda-tarafa
added a commit
to googleapis/google-api-dotnet-client
that referenced
this pull request
Feb 22, 2024
- The universe domain may be specified through the client service initializer. - A helper method to calculate effective URIs that takes into account the universe domain. This method is to be used by generated code, see googleapis/gapic-generator-csharp#746. - The universe domain is passed as a request option for the credential to validate against when intercepting and before setting the token.
amanda-tarafa
force-pushed
the
discovery-universe-domain
branch
from
d25cb64 to
f537a7a
Compare
amanda-tarafa
commented
Feb 22, 2024
There was a problem hiding this comment.
I guess there's a slight memory footprint increase here with more fields rather than calculated properties, but I very much doubt that it'll be significant.
Yes, I did choose this actively. Calculating the properties now implies replacing "googleapis.com" with the custom domain, if there is one. And that happens for every service call, so I figured having the fields was slightly better. Happy to reconsider. Ultimately I think neither is going to make much of a difference though.
| .WithBlockBody(resourceProperties.Zip(Resources).Select(pair => pair.First.Assign(New(ctx.Type(pair.Second.Typ))(This))).ToArray()) | ||
| .WithBlockBody( | ||
| resourceProperties.Zip(Resources).Select(pair => pair.First.Assign(New(ctx.Type(pair.Second.Typ))(This))) | ||
| .Append(baseUriProperty.Assign(Call("GetEffectiveUri")(IdentifierName("BaseUriOverride"), baseUriValue))) |
Once we have released the generator and updated Discovery based generation to use the new version of the generator, we'll be able to update the Google.Apis.Discovery.v1 (1.66.0 -> 1.67.0) dependency in Google.Api.Generator.Rest instead. See b/326453788
Note that this is not an extension on another syntax element, we have plenty of those, nor a this-qualified method call. This generates a method call to an instance method without the this qualifier.
Generate code that calculates URIs taking the universe domain into account.
amanda-tarafa
force-pushed
the
discovery-universe-domain
branch
from
f537a7a to
31904ad
Compare
|
I've updated to latest GAX and Google.Apis (tests only) latests. I'll merge on green. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This has a dependency on googleapis/google-api-dotnet-client#2675 because generated code uses the methods defined there. This PR's presubmits won't be green until we have merged and released googleapis/google-api-dotnet-client#2675.