-
Notifications
You must be signed in to change notification settings - Fork 554
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
Serve openapi spec #317
Serve openapi spec #317
Conversation
91781e6
to
5f5aa15
Compare
Thanks for the PR, is there anything else left to do there mainly asking as its draft PR? I will give it a try if this fixes the bug for us as well, thanks! |
I get the following errors when trying to build:
Do you mind just detailing the steps you used to build it, thanks! |
Sorry, you need to change the I added
and then it should just work ™️ . No idea why this is actually gone. |
Thanks that worked compiling it, but I still get apiserver errors in the log, maybe I have a wrong setup can you maybe share yours if those do not happen to you? Thanks!
|
Sorry, I was mainly testing with the test-adapter here kubernetes-sigs/custom-metrics-apiserver#65 (comment), but will try to find some time to test this adapter directly. |
@johanneswuerbach no worries, feel free to reach out if you need any help or me to test anything! Thanks again! |
Hey @lilic I pushed a version pointing directly to my fork and it is working for me (no more errors, and output with |
@johanneswuerbach yay, thanks for the ping, will test it out today!! 🎉 |
Tested, and no more errors in apiserver logs! Once is merged kubernetes-sigs/custom-metrics-apiserver#65 we can make this nondraft, correct or are there any other blockers? Thanks again! |
No, we can either merge this PR directly or first #316 and then this PR once kubernetes-sigs/custom-metrics-apiserver#65 is merged. |
@johanneswuerbach if you feel this can be merged, please let me know and thank you for the contribution! Please squash commits in logical packets please if possible. If possible please make separate commits for:
|
@s-urbaniak, sure. Should #316 be merged first or do you prefer to close 316 and make all changes only in this PR? |
@johanneswuerbach I think doing it in one PR does the trick and reduces cross-PR coordination. So closing #316 and doing everything here is fine. Thank you so much!!!! 🎉 |
d21a199
to
7a1bdec
Compare
@s-urbaniak rebased, does the split make sense or should I split the commits differently? |
@johanneswuerbach this looks perfect, I am happy to merge, but I am just asking one last thing if you don't mind :-) We are in the process of moving this project to kubernetes-sigs (see kubernetes/org#2182). So it would be great if you could sign the CNCF CLA (see kubernetes/org#2182 (comment)) so we can continue with a smooth transition :-) Thank you! 🎉 |
I've been contributing to kubernetes before, so the CLA should already be signed kubernetes/enhancements#2022 Or is there something different here? |
Ah, no this looks perfect then, merging, and thank you so much! 🎉 |
@s-urbaniak Would you please release a new version as well? 🙏 |
@renan yes, I just released 0.8.0 yesterday https://github.com/DirectXMan12/k8s-prometheus-adapter/releases/tag/v0.8.0 :-) |
Hi @s-urbaniak could you please take a look at the following issue of the release you've mentioned Thank you in advance. |
Use kubernetes-sigs/custom-metrics-apiserver#65 to serve an openapi spec.
Based on #316
Fixes #257
Fixes #254