-
Notifications
You must be signed in to change notification settings - Fork 9.8k
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
Update get provider schema to include state store information #36834
Update get provider schema to include state store information #36834
Conversation
internal/grpcwrap/provider.go
Outdated
@@ -73,6 +74,7 @@ func (p *provider) GetSchema(_ context.Context, req *tfplugin5.GetProviderSchema | |||
Block: convert.ConfigSchemaToProto(dat.Body), | |||
} | |||
} | |||
// Do we add state stores here? Why aren't ephemeral resources here? |
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.
This confused me, and I'll take another look tomorrow
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.
Added in 81b44cf
I think ephemeral resources were omitted here accidentally, and that didn't matter because it's a testing resource for E2E tests.
I think this is at an ok level of completion for merging into the prototyping branch (assuming no glaring errors). Having this implemented unblocks any code that may use the provider schema response data. |
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.
Overall LGTM, just two very minor comments/questions.
6ff3410
to
b54b90e
Compare
Co-authored-by: Radek Simko <[email protected]>
5742c22
to
122ca70
Compare
fd74a25
into
radek/pluggable-backends-protocol
This PR updates the GetProviderSchema/GetSchema response so that it includes info about state stores.