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

ariels' version of: Upgrade version of oapi-codegen #7532

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

arielshaqed
Copy link
Contributor

ariels' version of #7493. This one is more for working and linking than it is for pulling!

Copy link

github-actions bot commented Mar 6, 2024

No linked issues found. Please add the corresponding issues in the pull request description.
Use GitHub automation to close the issue when a PR is merged

@arielshaqed arielshaqed changed the title Upgrade version of oapi-codegen ariels' version of: Upgrade version of oapi-codegen Mar 6, 2024
@jamietanna
Copy link

Looks like the current failures are due to kin-openapi changes in between the versions we're using - I can see pkg/api/tmpl/inline.tmpl may need an update, but once updating it it doesn't regenerate any of the templates we've got

@arielshaqed
Copy link
Contributor Author

Looks like the current failures are due to kin-openapi changes in between the versions we're using - I can see pkg/api/tmpl/inline.tmpl may need an update, but once updating it it doesn't regenerate any of the templates we've got

Yeah, I figured those out. Now it's broken because the new codegen the OpenAPI 3.1 interpretation that forbids DELETE with a body. Can I override that setting somehow?

@Jonathan-Rosenberg
Copy link
Contributor

Jonathan-Rosenberg commented Mar 21, 2024

@arielshaqed the reason for the failures was due to the incorrect placement of the template files. It changed a lot since the old 1.5.6 days...

Mind the change under the: pkg/api/apigen/tmpl path

Copy link

github-actions bot commented Apr 10, 2024

🎊 PR Preview 9ebd118 has been successfully built and deployed to https://treeverse-lakeFS-preview-pr-7532.surge.sh

🕐 Build time: 0.012s

🤖 By surge-preview

Copy link

github-actions bot commented Apr 10, 2024

E2E Test Results - DynamoDB Local - Local Block Adapter

13 passed

jamietanna and others added 10 commits April 10, 2024 23:00
To get the latest features and bug fixes, we should bump to the latest
version of the library.
Still does not compile: generated type of AbortPresignMultipartUpload method
changed, and it is not clear where to get the body!

```compilation
pkg/api/serve.go:59:35: cannot use controller (variable of type *Controller) as apigen.ServerInterface value in argument to apigen.HandlerFromMuxWithBaseURL: *Controller does not implement apigen.ServerInterface (wrong type for method AbortPresignMultipartUpload)
		have AbortPresignMultipartUpload("net/http".ResponseWriter, *"net/http".Request, apigen.AbortPresignMultipartUpload, string, string, string, apigen.AbortPresignMultipartUploadParams)
		want AbortPresignMultipartUpload("net/http".ResponseWriter, *"net/http".Request, string, string, string, apigen.AbortPresignMultipartUploadParams)
```
The new Kin generator actually enforces examples are correct.  This is a
Good Thing.
Apparently Kin now enforces even more OpenAPI rules.
@arielshaqed arielshaqed marked this pull request as ready for review April 10, 2024 20:01
@arielshaqed arielshaqed added area/API Improvements or additions to the API improvement infrastructure build, deploy and release processes tech-debt go Pull requests that update Go code and removed pr/do-not-merge labels Apr 10, 2024
@arielshaqed arielshaqed added the include-changelog PR description should be included in next release changelog label Apr 11, 2024
@arielshaqed arielshaqed marked this pull request as draft April 11, 2024 12:47
The new generated code no longer aliases some types.  This manifests
primarily in all pagination parameters, which are now actual strings and
ints.
Copy link

This PR is now marked as stale after 30 days of inactivity, and will be closed soon. To keep it, mark it with the "no stale" label.

@github-actions github-actions bot added the stale label May 12, 2024
@arielshaqed arielshaqed added the no stale Using this label will prevent items from being marked as stale label May 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/API Improvements or additions to the API go Pull requests that update Go code improvement include-changelog PR description should be included in next release changelog infrastructure build, deploy and release processes no stale Using this label will prevent items from being marked as stale stale tech-debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants