feat: graphql types update for managed resource output secret reference#290
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey nxtcoder17 - Here's my review!
General suggestions:
- Ensure that newly added resolver functions are fully implemented to prevent runtime panics.
- Consider providing mock implementations for resolver functions that are not yet implemented to facilitate comprehensive testing.
- Review the GraphQL schema changes to ensure they align with the intended data representation and access patterns.
Thanks for using Sourcery. We offer it for free for open source projects and would be very grateful if you could help us grow. If you like it, would you consider sharing Sourcery on your favourite social media? ✨
| func (r *k8s__io___api___core___v1__SecretResolver) Data(ctx context.Context, obj *v11.Secret) (map[string]interface{}, error) { | ||
| panic(fmt.Errorf("not implemented: Data - data")) |
There was a problem hiding this comment.
issue (llm): Implementing resolvers with panic for 'not implemented' is risky in production code. It's crucial to either implement these methods or handle them gracefully to avoid runtime panics.
|
|
||
| // Data is the resolver for the data field. | ||
| func (r *k8s__io___api___core___v1__SecretResolver) Data(ctx context.Context, obj *v11.Secret) (map[string]interface{}, error) { | ||
| panic(fmt.Errorf("not implemented: Data - data")) |
There was a problem hiding this comment.
issue (llm): It appears that the resolver functions for Data, StringData, and Type are not implemented yet and are currently placeholders that will panic. This is a critical issue for testing because it means that any tests that would interact with these resolvers would fail. Implementing these resolvers or at least providing mock implementations that do not panic would be necessary for comprehensive testing.
|
|
||
| // IsReadyOnly is the resolver for the isReadyOnly field. | ||
| func (r *secretResolver) IsReadyOnly(ctx context.Context, obj *entities.Secret) (bool, error) { | ||
| panic(fmt.Errorf("not implemented: IsReadyOnly - isReadyOnly")) |
There was a problem hiding this comment.
issue (llm): Similar to the previous comment, the IsReadyOnly resolver function is not implemented and will panic. This needs to be addressed for comprehensive testing of the secret resolvers. Implementing or mocking this function is necessary to ensure that tests can cover this functionality without causing runtime panics.
|
|
||
| SyncedOutputSecretRef *corev1.Secret `json:"syncedOutputSecretRef" graphql:"ignore" struct-json-path:",ignore-nesting"` | ||
| // SyncedOutputSecretRef *corev1.Secret `json:"syncedOutputSecretRef" graphql:"noinput" struct-json-path:",ignore-nesting"` | ||
| SyncedOutputSecretRef *corev1.Secret `json:"syncedOutputSecretRef" graphql:"noinput"` |
There was a problem hiding this comment.
suggestion (llm): The change in SyncedOutputSecretRef to include it with graphql:"noinput" suggests a modification in how this field is treated. It's important to ensure that there are tests to verify that this field is correctly ignored or processed in GraphQL queries and mutations as intended. If such tests are not present, adding them would be crucial to validate the behavior of SyncedOutputSecretRef in the context of GraphQL operations.
nxtcoder17
force-pushed
the
fix/managedresource-output-secret-ref
branch
from
7d71fae to
1837f31
Compare
nxtcoder17
force-pushed
the
fix/managedresource-output-secret-ref
branch
from
1837f31 to
79b23cf
Compare
…ret-ref feat: graphql types update for managed resource output secret reference
1 participant
No description provided.