search

openservicemesh / osm

Open Service Mesh (OSM) is a lightweight, extensible, cloud native service mesh that allows users to uniformly manage, secure, and get out-of-the-box observability features for highly dynamic microservice environments.
https://openservicemesh.io/
Apache License 2.0
2.59k stars 277 forks source link

fix(): resolve lint error from unchecked AddEventHandler return values #5306

Closed jaellio closed 1 year ago

jaellio commented 1 year ago

Description: Fixes lint error introduced in #5283

When upgrading helm to 3.11.1, the k8.io client-go package was also updated to v0.26.3. In v0.26 the API for AddEventHandler was updated to return an error and registration handle to enable individual handler cancelation.

Please answer the following questions with yes/no.

  1. Does this change contain code from or inspired by another project? No

    • Did you notify the maintainers and provide attribution?

  2. Is this a breaking change? No

  3. Has documentation corresponding to this change been updated in the osm-docs repo (if applicable)? No

steeling commented 1 year ago

i'm surprised we don't have the linter enabled to ensure errors are checked. We should check and do something with those errors most likely

keithmattix commented 1 year ago

In general yes, but for a backport, we should probably persist existing behavior right?

steeling commented 1 year ago

In general yes, but for a backport, we should probably persist existing behavior right?

I'll just point out this is more than a backport if its making it into main

keithmattix commented 1 year ago

In general yes, but for a backport, we should probably persist existing behavior right?

I'll just point out this is more than a backport if its making it into main

Oh I misread which PR this is. That's a fair point...in that case, I don't have a strong preference for which direction we go this PR

jaellio commented 1 year ago

In general yes, but for a backport, we should probably persist existing behavior right?

I'll just point out this is more than a backport if its making it into main

Agree, it isn't much code to change to add the error handling. I've updated the PR to include these changes.