-
Notifications
You must be signed in to change notification settings - Fork 594
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
chore: read admin api response body from error #6244
Conversation
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.
🏅
So the next step is to just return only a particular error or nil
from ReloadDeclarativeRawConfig
? 🤔
Well, given we haven't released go-kong v1 yet, we can release a breaking change I believe 🤔 Kong/go-kong#453. Let's hold on with merging this PR and release go-kong first so it's done in a single step. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6244 +/- ##
======================================
Coverage ? 73.6%
======================================
Files ? 200
Lines ? 19912
Branches ? 0
======================================
Hits ? 14673
Misses ? 4253
Partials ? 986 ☔ View full report in Codecov by Sentry. |
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.
🏆
What this PR does / why we need it:
Don't rely on
ReloadDeclarativeRawConfig
to return the error body whenerr != nil
as it might be considered a Go anti-pattern to use variables when an error is returned. Instead, rely on theAPIError
being returned in cases where the error body is available.Which issue this PR fixes:
Follow up to #6237 (comment).