internal: Migrate ValidateProviderConfig RPC to fromproto6, fwserver, and toproto6 #320
Conversation
… and toproto6 Reference: #215 One particular quirk that this implementation shows is that it is not entirely possible to avoid working with `tfsdk`/`fwserver` bits in `proto6server` as the current data types within the framework (`Config`, `Plan`, `State`) require the `Schema` so data can be appropriately converted from the protocol to the framework. To support this effort and prevent repetitively executing the provider defined `GetSchema` method, a `fwserver` method is introduced that caches the `Schema` and `Diagnostics` results on first use. This methodology can also be applied in the future to resolve #299.
bflad
changed the title
internal/fwserver/server.go
Outdated
|
|
||
| if s.providerSchema != nil { | ||
| s.providerSchemaMutex.Unlock() | ||
| return s.providerSchema, nil |
There was a problem hiding this comment.
Should this return also s.ProviderSchemaDiags?
Unless the intention is to report potential diag-reported warnings/errors, only the first time?
There was a problem hiding this comment.
Yes, it should, thanks for noticing!
internal/fwserver/server.go
Outdated
| s.providerSchemaMutex.Lock() | ||
|
|
||
| if s.providerSchema != nil { | ||
| s.providerSchemaMutex.Unlock() |
There was a problem hiding this comment.
To ensure future code doesn't break the needs of the mutex to be unlocked, I have seen a slightly different usage pattern (that I like). Something like:
s.ProviderSchemaMutex.Lock()
defer s.ProviderSchemaMutex.Unlock()
...
This way, whatever branching of the logic we take (the memoized one or not), the locking bit is handled safely and consistently.
There was a problem hiding this comment.
Thank you for this suggestion, it's a great one and one I'll implement!
…e returned in (Server).ProviderSchema()
|
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. |
Reference: #215
One particular quirk that this implementation shows is that it is not entirely possible to avoid working with
tfsdk/fwserverbits inproto6serveras the current data types within the framework (Config,Plan,State) require theSchemaso data can be appropriately converted from the protocol to the framework. To support this effort and prevent repetitively executing the provider definedGetSchemamethod, afwservermethod is introduced that caches theSchemaandDiagnosticsresults on first use. This methodology can also be applied in the future to resolve the existing #299.