enable schema caching and add get_provider_schema_optional server capability
#33486
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
bflad
reviewed
Jul 6, 2023
jbardin
changed the title
get_provider_schema_optional server capability
bflad
approved these changes
Jul 7, 2023
jbardin
force-pushed
the
jbardin/schema-cache-proto
branch
from
f63d2b5
to
6429042
Compare
|
Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch. |
bflad
added a commit
that referenced
this pull request
Aug 24, 2023
Reference: #33486 This is a followup to the new provider server capability to make the `GetProviderSchema` RPC optional. While this server capability would perform its intended function when directly talking to a single provider server SDK implementation, provider servers using terraform-plugin-mux need a methodology for the mux server to determine the available resource types of each underlying provider server to properly route resource-specific RPCs. Since the only methodology available to the mux server would be calling the `GetProviderSchema` RPC to each of underlying provider servers, any memory optimization of core caching would be lost. The choice of adding a new RPC instead of adjusting the existing `GetProviderSchema` RPC with additional request information, such as "only list the type names and not the schema information in the response", is two-fold: - Prevents the introduction of conditional logic for the existing RPC. - Clearly delineates the purpose of the RPC and can be documented easier. The choice of adding this to the existing provider service is two-fold: - Implementing a separate protocol and/or service only on the provider side of the protocol would be a novel design change. This small of a change does not warrant the potential research and testing effort that would be associated with that implementation. - While the core implementation will not use the new RPC immediately, there is no reason why it should be restricted from doing so in the future if a valid use case surfaces. Other ecosystem tools, beyond terraform-plugin-mux, can also potentially benefit from the lightweight RPC now. This is changing the 5.4 and 6.4 protocol versions following the guidance of this comment in the definition files, since it directly relates to the prior intention of the new minor versions: ```protobuf // Note that only the proto files included in a release tag of Terraform are // official protocol releases. Proto files taken from other commits may include // incomplete changes or features that did not make it into a final release. // In all reasonable cases, plugin developers should take the proto file from // the tag of the most recent release of Terraform, and not from the main // branch or any other development branch. ``` As with any Protocol Buffers definition update, protocol compatibility is guaranteed within a major version, however generated protocol source code compatibility is not guaranteed. In this case, implementing the new RPC method in protocol wrapper types and the moving of the `ServerCapabilities` message to the top namespace are considered acceptable changes.
bflad
added a commit
that referenced
this pull request
Aug 24, 2023
Reference: #33486 This is a followup to the new provider server capability to make the `GetProviderSchema` RPC optional. While this server capability would perform its intended function when directly talking to a single provider server SDK implementation, provider servers using terraform-plugin-mux need a methodology for the mux server to determine the available resource types of each underlying provider server to properly route resource-specific RPCs. Since the only methodology available to the mux server would be calling the `GetProviderSchema` RPC to each of underlying provider servers, any memory optimization of core caching would be lost. The choice of adding a new RPC instead of adjusting the existing `GetProviderSchema` RPC with additional request information, such as "only list the type names and not the schema information in the response", is two-fold: - Prevents the introduction of conditional logic for the existing RPC. - Clearly delineates the purpose of the RPC and can be documented easier. The choice of adding this to the existing provider service is two-fold: - Implementing a separate protocol and/or service only on the provider side of the protocol would be a novel design change. This small of a change does not warrant the potential research and testing effort that would be associated with that implementation. - While the core implementation will not use the new RPC immediately, there is no reason why it should be restricted from doing so in the future if a valid use case surfaces. Other ecosystem tools, beyond terraform-plugin-mux, can also potentially benefit from the lightweight RPC now. This is changing the 5.4 and 6.4 protocol versions following the guidance of this comment in the definition files, since it directly relates to the prior intention of the new minor versions: ```protobuf // Note that only the proto files included in a release tag of Terraform are // official protocol releases. Proto files taken from other commits may include // incomplete changes or features that did not make it into a final release. // In all reasonable cases, plugin developers should take the proto file from // the tag of the most recent release of Terraform, and not from the main // branch or any other development branch. ``` As with any Protocol Buffers definition update, protocol compatibility is guaranteed within a major version, however generated protocol source code compatibility is not guaranteed. In this case, implementing the new RPC method in protocol wrapper types and the moving of the `ServerCapabilities` message to the top namespace are considered acceptable changes.
bflad
added a commit
that referenced
this pull request
Aug 24, 2023
Reference: #33486 This is a followup to the new provider server capability to make the `GetProviderSchema` RPC optional. While this server capability would perform its intended function when directly talking to a single provider server SDK implementation, provider servers using terraform-plugin-mux need a methodology for the mux server to determine the available resource types of each underlying provider server to properly route resource-specific RPCs. Since the only methodology available to the mux server would be calling the `GetProviderSchema` RPC to each of underlying provider servers, any memory optimization of core caching would be lost. The choice of adding a new RPC instead of adjusting the existing `GetProviderSchema` RPC with additional request information, such as "only list the type names and not the schema information in the response", is two-fold: - Prevents the introduction of conditional logic for the existing RPC. - Clearly delineates the purpose of the RPC and can be documented easier. The choice of adding this to the existing provider service is two-fold: - Implementing a separate protocol and/or service only on the provider side of the protocol would be a novel design change. This small of a change does not warrant the potential research and testing effort that would be associated with that implementation. - While the core implementation will not use the new RPC immediately, there is no reason why it should be restricted from doing so in the future if a valid use case surfaces. Other ecosystem tools, beyond terraform-plugin-mux, can also potentially benefit from the lightweight RPC now. This is changing the 5.4 and 6.4 protocol versions following the guidance of this comment in the definition files, since it directly relates to the prior intention of the new minor versions: ```protobuf // Note that only the proto files included in a release tag of Terraform are // official protocol releases. Proto files taken from other commits may include // incomplete changes or features that did not make it into a final release. // In all reasonable cases, plugin developers should take the proto file from // the tag of the most recent release of Terraform, and not from the main // branch or any other development branch. ``` As with any Protocol Buffers definition update, protocol compatibility is guaranteed within a major version, however generated protocol source code compatibility is not guaranteed. In this case, implementing the new RPC method in protocol wrapper types and the moving of the `ServerCapabilities` message to the top namespace are considered acceptable changes.
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Add a
get_provider_schema_optionalserver capability, indicating that the provider will operate normally without calling theGetProviderSchemamethod, and a cached version of the schema can be used. We can then enable the global cache in core, allowing us to only callGetProviderSchemaonce per command, and reducing the number of times a provider needs to be invoked to obtain the schema.The global schema cache allows multiple benefits: