-
Notifications
You must be signed in to change notification settings - Fork 115
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
Add k8s client tuning settings to Provider #1748
Conversation
Does the PR have any schema changes?Looking good! No breaking changes found. |
2 similar comments
Does the PR have any schema changes?Looking good! No breaking changes found. |
Does the PR have any schema changes?Looking good! No breaking changes found. |
@@ -1,4 +1,4 @@ | |||
// Copyright 2016-2020, Pulumi Corporation. | |||
// Copyright 2016-2021, Pulumi Corporation. |
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.
You can ignore most of the changes in this file. I refactored the declaration into separate variables, but it's not a functional change.
"devel": { | ||
TypeSpec: pschema.TypeSpec{ | ||
Type: "boolean", | ||
var kubeClientSettings = pschema.ComplexTypeSpec{ |
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.
Here's the new type.
@@ -231,6 +241,7 @@ func PulumiSchema(swagger map[string]interface{}) pschema.PackageSpec { | |||
|
|||
csharpNamespaces := map[string]string{ | |||
"helm.sh/v3": "Helm.V3", | |||
"": "Provider", |
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.
A little confused why this was needed? I see that Provider.cs already existed right?
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.
I couldn't get the C# SDK to generate correctly without that. The compatibility logic is causing problems without it.
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.
Do we have a tracking bug for this in codegen?
Does the PR have any schema changes?Looking good! No breaking changes found. |
- Provide options to configure burst and QPS - Configure the provider client to use these settings
92cea29
to
327e7e1
Compare
Does the PR have any schema changes?Looking good! No breaking changes found. |
@lukehoban @viveklak it looks like the provider codegen for Node.js doesn't handle the nested object and I am not familiar enough with the codegen to understand what would need to change. Discovered as I investigated why the env var did not have any impact on |
Oh, I see the problem. I filed pulumi/pulumi#8132 after finishing this PR, and forgot to update it. The Node SDK isn't handling the env vars properly, so it will only work with a stack config or arguments to a Provider. https://github.com/pulumi/pulumi-kubernetes/blob/master/CHANGELOG.md#380-october-6-2021 has notes on setting nested Provider values that would be similar. I'll get the env vars fixed soon, but you can test it in the meantime with the other options. |
Proposed changes
Related issues (optional)
Fix #1704