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 verify-codegen #695

Merged
merged 2 commits into from
May 31, 2023
Merged

Conversation

t0rr3sp3dr0
Copy link
Contributor

Fix verify-codegen.sh and run make codegen to resync generated code.

Checklist

Fixes #694

Signed-off-by: Pedro Tôrres <pedrotorres@microsoft.com>
Signed-off-by: Pedro Tôrres <pedrotorres@microsoft.com>
@t0rr3sp3dr0 t0rr3sp3dr0 requested a review from a team as a code owner May 31, 2023 23:36
@t0rr3sp3dr0
Copy link
Contributor Author

@JorTurFer, it seems this was broken for a while. When we introduced multi-host support, the generated code became outdated but all the tests passed.

@JorTurFer JorTurFer enabled auto-merge (squash) May 31, 2023 23:40
@JorTurFer
Copy link
Member

@JorTurFer, it seems this was broken for a while. When we introduced multi-host support, the generated code became outdated but all the tests passed.

Lol... Interesting because I added an e2e test which tests multi-host use case and it works

@JorTurFer
Copy link
Member

@JorTurFer JorTurFer merged commit 70a0335 into kedacore:main May 31, 2023
19 checks passed
@JorTurFer
Copy link
Member

JorTurFer commented May 31, 2023

Tomorrow I'll check it but @t0rr3sp3dr0 ,could the generated code have not changed? Now with your fix the generated code hasn't changed

@t0rr3sp3dr0
Copy link
Contributor Author

t0rr3sp3dr0 commented May 31, 2023

It was just the check that was broken, make generate was producing the correct code. But if someone forgot to run it after changing the API structs, make verify would always succeed.

The only part that was actually outdated is the DeepCopy implementation for HTTPScaledObjectSpec: https://github.com/kedacore/http-add-on/pull/695/files#diff-8cc502f3f92b781bd8ad605d7e055e28f8342083518e951afc0de2ab174f1adb.

This is not used by the code on the main branch, that's why the E2E tests succeeded. But it affected the new routing table implementation as it relies on that method, that's why I noticed the problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

make verify is broken
2 participants