provider functions can only return an error #34603
Conversation
jbardin
force-pushed
the
jbardin/remove-provider-funtion-warnings
branch
from
44f5958
to
6817e76
Compare
jbardin
force-pushed
the
jbardin/remove-provider-funtion-warnings
branch
from
6817e76
to
c34c53e
Compare
jbardin
changed the title
There was a problem hiding this comment.
It seems to me that this is essentially a one-way door because if we did want to add back support for arbitrary diagnostics later then we'd need to find some way to stuff at least the error ones into this error field for backward-compatibility with older Terraform Core versions that wouldn't know to look for the diagnostics. We'd also not be able to rely on everyone seeing any warnings that a function returns, which would limit the situations where it would be useful to return them.
Given that, I think we should just go all-in on this thing being shaped like a decision between plain error and cty/function.ArgError, and thus reduce this to just a single message and optional function_argument, rather than having it be sorta-kinda-diagnostic-shaped. 🤔
I would then expect the plugin framework to represent functions in a similar way to how cty/function does, where they return error and then it type-asserts for a specific error type to decide whether to populate that function_argument field.
|
I don't think this is a one-way door per say, but I do think changing the function call site for all existing functions to handle diagnostics is much harder than re-extending this particular function implementation on its own. I agree that having this be a half-diagnostic doesn't really benefit us, so we might as well make it a simple error with optional |
The call site for language functions doesn't currently have a way to handle complex diagnostics, so rather than appear to support them in the protocol we remove the concepts of diagnostics for now. We do however retain the argument index fields, which we can wrap in a function.ArgError and get a little more precise hcl diagnostic from expression.
jbardin
force-pushed
the
jbardin/remove-provider-funtion-warnings
branch
from
c34c53e
to
a8701f6
Compare
… with function error Reference: hashicorp/terraform#34603 The next version of the plugin protocol (5.5/6.5) includes support for provider defined functions. This change modifies the response returned from the CallFunction RPC, replacing diagnostics with function error.
… with function error (#380) * tfprotov5+tfprotov6: Replace usage of diagnostics in CallFunction RPC with function error Reference: hashicorp/terraform#34603 The next version of the plugin protocol (5.5/6.5) includes support for provider defined functions. This change modifies the response returned from the CallFunction RPC, replacing diagnostics with function error. * Fix protoc version * Add changelog * Removing function argument from diagnostics * Renaming param from msg to text for function errorr * Adding change log
|
Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch. |
|
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. |
The call site for language functions doesn't currently have a way to handle complex diagnostics, so rather than appear to support them in the protocol we remove the concept of diagnostics for now. We do however retain the argument index field, which we can wrap in a
function.ArgErrorto get a little more precisehcldiagnostic from the expression.