Skip to content
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

Handle errors during ReadDataApply #2174

Closed
wants to merge 1 commit into from

Conversation

adrien-f
Copy link

As seen in the Protobuf, ReadDataSource_Response can contain diagnostics errors.

As seen in the Protobuf, ReadDataSource_Response can contain
diagnostics errors.

Signed-off-by: Adrien Fillon <[email protected]>
@VenelinMartinov
Copy link
Contributor

Hi @adrien-f, thanks for contributing.

The change looks sensible but I am wondering if you are using this code and what your use case here is?

I am not aware of any uses of it in pulumi - we have a better supported way of bridging providers. Was that insufficient for your use case or is there some other limitation I am not aware of?

@adrien-f
Copy link
Author

Hey @VenelinMartinov !

I can understand this PR coming out of nowhere 😅 I was playing around the tfshim library to learn how it works and ran into this issue on my way.

@VenelinMartinov
Copy link
Contributor

@adrien-f that's great, glad you are finding it interesting and hope you haven't hit too many issues!

The tfplugin5 interface is a bit abandoned since it isn't really used internally - most of our providers are using the sdkv2 interface:

func NewProvider(p *schema.Provider, opts ...providerOption) shim.Provider {

There's also the pf bridge which is somewhat separate - it acts on the v6 version of the TF protocol:

type provider struct {
(ex https://github.com/pulumi/pulumi-random)

@adrien-f
Copy link
Author

Many thanks for the pointers 👀

@mjeffryes
Copy link
Member

we might be able to get rid of tfshim/tfplugin5. Might be worth soliciting for community feedback to find out if anyone is using it or if we could just drop it with no impact.

@VenelinMartinov

This comment was marked as outdated.

@guineveresaenger
Copy link
Contributor

Hi @adrien-f - thanks again for bringing this to our attention.

We have decided to deprecate this particular implementation of tfplugin via #2197.

Thank you for making us aware.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants