Skip to content

Conversation

@bendbennett
Copy link
Contributor

Closes: #176

@bendbennett bendbennett added the bug Something isn't working label Jul 13, 2023
@bendbennett bendbennett added this to the v0.11.2 milestone Jul 13, 2023
@bendbennett bendbennett requested a review from a team as a code owner July 13, 2023 14:59
@bflad bflad self-assigned this Jul 13, 2023
Comment on lines 166 to 177
type TestServerDiags struct {
*TestServer
Diagnostics []*tfprotov5.Diagnostic
}

func (s *TestServerDiags) GetProviderSchema(ctx context.Context, req *tfprotov5.GetProviderSchemaRequest) (*tfprotov5.GetProviderSchemaResponse, error) {
resp, err := s.TestServer.GetProviderSchema(ctx, req)

resp.Diagnostics = append(resp.Diagnostics, s.Diagnostics...)

return resp, err
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of setting up a new test server type, how do you feel about updating the existing TestServer to store a full GetProviderSchemaResponse *tfprotov5.GetProviderSchemaResponse field similar to PrepareProviderConfigResponse *tfprotov5.PrepareProviderConfigResponse?

We can migrate all the other GetProviderSchema related fields to be under that as well, rather than individually calling them out:

DataSourceSchemas map[string]*tfprotov5.Schema	ProviderMetaSchema *tfprotov5.Schema	ProviderSchema *tfprotov5.Schema	ResourceSchemas map[string]*tfprotov5.Schema	ServerCapabilities *tfprotov5.ServerCapabilities 

I'm going to peek at the other RPC logic and create an issue if it makes sense to do something similar for the other RPCs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call. Have refactored.

Comment on lines 2 to 3
body: 'tf5muxserver: Include diagnostics returned from `server.GetProviderSchema()`
in `muxServer.GetProviderSchemaResponse`'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I think we can tailor this message to developers without talking about the internal details, e.g.

tf5muxserver: Ensure GetProviderSchema RPC diagnostics are properly returned to Terraform

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, have updated.

@bendbennett bendbennett changed the title Error diagnostics are not surfaced in the CLI when default is used for non-computed attribute Ensure GetProviderSchema Response Error/Diagnostics are Unit Tested Jul 14, 2023
Copy link
Contributor

@bflad bflad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great to me 🚀 Thank you for hunkering down to add the response object in there

@bendbennett
Copy link
Contributor Author

I'll try and circle back to take of Ensure ConfigureProvider Response Diagnostics are Unit Tested in the near future.

@bendbennett bendbennett merged commit 3c22b0a into main Jul 14, 2023
@bendbennett bendbennett deleted the bendbennett/issues-176 branch July 14, 2023 12:59
@github-actions
Copy link

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.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bug Something isn't working

2 participants