Skip to content
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

fix(api): use Unstructured to return raw response #1903

Merged
merged 1 commit into from
Apr 30, 2024

Conversation

hiddeco
Copy link
Contributor

@hiddeco hiddeco commented Apr 29, 2024

Fixes: #1899

This pull request ensures that the raw object data returned by the Kargo API matches the data from the Kubernetes API server. Which, unlike the JSON and YAML serializers, purges empty structures.

Copy link

netlify bot commented Apr 29, 2024

Deploy Preview for docs-kargo-akuity-io ready!

Name Link
🔨 Latest commit 418bf14
🔍 Latest deploy log https://app.netlify.com/sites/docs-kargo-akuity-io/deploys/662f8638198914000851955d
😎 Deploy Preview https://deploy-preview-1903.kargo.akuity.io
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

codecov bot commented Apr 29, 2024

Codecov Report

Attention: Patch coverage is 78.50467% with 23 lines in your changes are missing coverage. Please review.

Project coverage is 46.84%. Comparing base (14522e0) to head (8ac4236).

❗ Current head 8ac4236 differs from pull request most recent head 418bf14. Consider uploading reports for the commit 418bf14 to get more accurate results

Files Patch % Lines
internal/api/get_analysistemplate_v1alpha1.go 62.96% 6 Missing and 4 partials ⚠️
internal/api/get_freight_v1alpha1.go 86.79% 4 Missing and 3 partials ⚠️
internal/api/get_analysisrun_v1alpha1.go 77.77% 3 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1903      +/-   ##
==========================================
+ Coverage   46.74%   46.84%   +0.09%     
==========================================
  Files         220      220              
  Lines       14411    14464      +53     
==========================================
+ Hits         6737     6776      +39     
- Misses       7374     7382       +8     
- Partials      300      306       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@hiddeco hiddeco force-pushed the improve-raw-response branch 3 times, most recently from 02444a0 to 20214fc Compare April 29, 2024 11:29
Signed-off-by: Hidde Beydals <hidde@hhh.computer>
@hiddeco hiddeco force-pushed the improve-raw-response branch from 20214fc to 418bf14 Compare April 29, 2024 11:36
Copy link
Contributor

@devholic devholic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

Copy link
Member

@jessesuen jessesuen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strange that this technique is necessary. Do you know why Kubernetes API doesn't suffer the same problem given that the authentication field is not a pointer?

@hiddeco
Copy link
Contributor Author

hiddeco commented Apr 29, 2024

@jessesuen I searched hard and looked at the Kubernetes SIG YAML library and the serializers in API machinery but could not figure out what causes the API to drop the empty objects. If I happen to run into the answer, I will revise what I introduced here.

@krancour krancour added this to the v0.6.0 milestone Apr 29, 2024
@hiddeco hiddeco added this pull request to the merge queue Apr 30, 2024
Merged via the queue into akuity:main with commit 73f06ef Apr 30, 2024
15 checks passed
@hiddeco hiddeco deleted the improve-raw-response branch April 30, 2024 05:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AnalysisRun RawFormat still shows omitempty fields
4 participants