cloud-barista / cb-tumblebug

Cloud-Barista Multi-Cloud Infra Management Framework
Apache License 2.0
56 stars 51 forks source link

Refactor label removal to use DeleteLabelObject function #1931

Closed yunkon-kim closed 4 days ago

yunkon-kim commented 4 days ago

This PR will mainly replace RemoveLabel() with DeleteLabelObject(). It's related to the last part of resource deletion functions.

@seokho-son

Could you please double-check whether the DeleteLabelObject() function is appropriate for the resource deletion function?

Which do you prefer between error or nil for the return value when the label object doesn't exist in RemoveLabel()?

close #1928

seokho-son commented 4 days ago

@yunkon-kim I think the DeleteLabelObject() function itself should return an error.

However, when using it, the function that calls DeleteLabelObject() can decide whether to propagate the error to its caller or not.

If the process involves important and time-consuming tasks, an error from deleting a label might be considered somewhat negligible.

yunkon-kim commented 4 days ago

(Sharing offline discussion)

  1. DeleteLabelObject() is suitable when deleting a resource.
  2. If labelObject is fetched in RemoveLabel() but there is no value, return an error and process it in the upper function.

There seems to be nothing to update :-)

yunkon-kim commented 4 days ago

/approve