Object Store Compatibility #56
Conversation
|
Signed-off-by: uzair <uzaxirr@gmail.com>
Signed-off-by: uzair <uzaxirr@gmail.com>
| Name: secretName, | ||
| }, connectionSecret) | ||
| if err != nil { | ||
| err = e.kube.Create(ctx, connectionSecret, &client.CreateOptions{}) |
There was a problem hiding this comment.
We should confirm that we got a NotExists error before we attempt to create
| return err | ||
| } | ||
| objectStore, err := e.civoClient.GetObjectStoreByName(os.Spec.Name) | ||
| if err != nil { |
There was a problem hiding this comment.
Can we not attempt a Delete directly? If we get a 404/NotFound error, we can consider that a successfull operation as resource doesn't exist- which is what Delete is responsible for.
Signed-off-by: uzair <uzaxirr@gmail.com>
Signed-off-by: uzair <uzaxirr@gmail.com>
| @@ -0,0 +1,20 @@ | |||
| package civoobjectstore | |||
There was a problem hiding this comment.
call this file as connection.go maybe? Just avoiding using utils
| ) | ||
|
|
||
| const ( | ||
| errNotCivoObjectStore = "managed resource is not a object store" |
There was a problem hiding this comment.
there are some useful methods in civogo to help with error assertion. https://github.com/civo/civogo/blob/master/errors.go#L291
| civoClient *civocli.CivoClient | ||
| } | ||
|
|
||
| // nolint |
There was a problem hiding this comment.
We should avoid using nolint and when we use it, we should have a reason noted for it.
pkg/civocli/cli.go
Outdated
| func (c *CivoClient) GetObjectStore(id string) (*civogo.ObjectStore, error) { | ||
| objectStore, err := c.civoGoClient.GetObjectStore(id) | ||
| if err != nil { | ||
| if strings.Contains(err.Error(), "ZeroMatchesError") { |
There was a problem hiding this comment.
Two things-
- Error assertion can be done with civogo
- We should asserting this in controller, not here.
Signed-off-by: Uzair Ali <72073401+uzaxirr@users.noreply.github.com>
Description of your changes
Fixes #
I have:
make reviewable testto ensure this PR is ready for review.How has this code been tested