-
Notifications
You must be signed in to change notification settings - Fork 17
List Resource QueryCheck #533
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
Conversation
Co-authored-by: stephybun <[email protected]>
Co-authored-by: stephybun <[email protected]>
Co-authored-by: stephybun <[email protected]>
…r/resource running
…butes, started to add length and length exact checks
…pleteData and refactor a little
…y checks to be consistent with existing statechecks
austinvalle
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good! Sounds like this is going to be a beta, so we can still come back and change any naming later if we so choose. All of the comments I left are nits that can be addressed/dismissed later
| // createDeltaString prints the map keys that are present in mapA and not present in mapB | ||
| func createDeltaString[T any, V any](mapA map[string]T, mapB map[string]V, msgPrefix string) string { | ||
| // CreateDeltaString prints the map keys that are present in mapA and not present in mapB | ||
| func CreateDeltaString[T any, V any](mapA map[string]T, mapB map[string]V, msgPrefix string) string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I'd say just move this function somewhere in the internal package, unless we want consumers to use it
| Diagnostics []*tfprotov6.Diagnostic | ||
| NewState tftypes.Value | ||
| NewIdentity *tftypes.Value | ||
| NewQuery tftypes.Value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Were these meant to be added to the create resp / delete request?
| type queryCheckSpy struct { | ||
| err error | ||
| called bool | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Doesn't look like this is used anywhere, perhaps it will eventually?
Related Issue
Continues #531
Description
Adds QueryCheck to plugin testing to go along with Query
Rollback Plan
Changes to Security Controls
Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.
No