search

kubecost / cluster-turndown

Automated turndown of Kubernetes clusters on specific schedules.
Apache License 2.0
259 stars 23 forks source link

send http responses in header rather than body #90

Closed nickcurie closed 2 months ago

nickcurie commented 2 months ago

Notes

nik-kc commented 2 months ago

Based on the code, it looks good that you have moved the error code from the response body to the response JSON.

But based on the original ticket, https://kubecost.atlassian.net/issues/ENG-751?filter=10157, there's a 500 error code. Are you able to repro that and see why that is happening? The ticket says cannot create a Turndown schedule.

Good catch, Alan, I didn't notice this. I echo this question.

nickcurie commented 2 months ago

Yeah if you see my comment here: https://kubecost.atlassian.net/browse/ENG-751?focusedCommentId=124848 the reason why we got the creation failure was due to the k8s api being unresponsive. I recall when I first started working on the ticket we needed to restart the pod because of this. The original issue was not reproducible after that point, so I just proceeded with modernizing parts of this repo since it was written 4 years ago and hasn't really been touched since

nik-kc commented 2 months ago

Yeah if you see my comment here: https://kubecost.atlassian.net/browse/ENG-751?focusedCommentId=124848 the reason why we got the creation failure was due to the k8s api being unresponsive. I recall when I first started working on the ticket we needed to restart the pod because of this. The original issue was not reproducible after that point, so I just proceeded with modernizing parts of this repo since it was written 4 years ago and hasn't really been touched since

I'm happy with that, I think this is good to merge. What say you, @avrodrigues5?

avrodrigues5 commented 2 months ago

Yeah if you see my comment here: https://kubecost.atlassian.net/browse/ENG-751?focusedCommentId=124848 the reason why we got the creation failure was due to the k8s api being unresponsive. I recall when I first started working on the ticket we needed to restart the pod because of this. The original issue was not reproducible after that point, so I just proceeded with modernizing parts of this repo since it was written 4 years ago and hasn't really been touched since

I'm happy with that, I think this is good to merge. What say you, @avrodrigues5?

Yup makes sense to me too. Thanks @nickcurie