-
-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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 Julia template for binary string #20355
base: master
Are you sure you want to change the base?
Conversation
Thanks @qiaojunfeng, I shall check this out this weekend. |
The effect of this change would be that if something is specified as binary format, it would now be generated as a I would like to understand more the reason why you feel we should have this changed in the openapi generated code? |
Thanks @tanmaykm! |
Thanks @qiaojunfeng, I think the issue here is missing check for the |
@qiaojunfeng Here's my recommended changes: https://github.com/OpenAPITools/openapi-generator/compare/master...tanmaykm:tan/fixfile?expand=1 |
@tanmaykm Thanks for the changes! Indeed using The reason comes from these:
function _oacinternal_upload_post(_api::DefaultApi, file::Vector{UInt8}; _mediaType=nothing)
_ctx = OpenAPI.Clients.Ctx(_api.client, "POST", _returntypes_upload_post_DefaultApi, "/upload", [])
OpenAPI.Clients.set_param(_ctx.file, "file", file) # type Vector{UInt8}
... https://github.com/qiaojunfeng/test-julia-OpenAPI/blob/7bed81b4a84fbdb93f2fcaf951e31903b053bb08/client/src/apis/api_DefaultApi.jl#L47-L49
for (_k,_v) in ctx.file
part = LibCURL.curl_mime_addpart(mime)
LibCURL.curl_mime_name(part, _k)
LibCURL.curl_mime_filedata(part, _v) Therefore, in my branch, I directly set function _oacinternal_upload_post(_api::DefaultApi, file::String; _mediaType=nothing)
_ctx = OpenAPI.Clients.Ctx(_api.client, "POST", _returntypes_upload_post_DefaultApi, "/upload", [])
OpenAPI.Clients.set_param(_ctx.file, "file", file) # type String
|
The best solution would not be to start writing tempfiles as a matter of routine. Just use curl_mime_data_cb() or curl_mime_data() instead of The path of least resistance is |
Although changing the current interface to something that uses tempfiles sounds to me like the wrong way to fix the bug, I also think it makes sense to use |
Thanks @qiaojunfeng for checking it out. I have adjusted my branch with some changes to generate the client methods that take file parameters as strings pointing to existing files. That should then work for cases where there is a real file. @qiaojunfeng would you be able to try that out? Here's the diff: https://github.com/OpenAPITools/openapi-generator/compare/master...tanmaykm:tan/fixfile?expand=1 Next (in seprate PRs?), we could make the necessary changes in OpenAPI.jl to handle both file names (as |
Fix missing check for the isBinary flag in some places of the templates for julialang. The code would now be generated as Vector{UInt8} uniformly for binary strings. Binary strings that are also marked as files get generated as `String` data type on the client. That is because the Julia client expects a file name to be passed there.
Thanks @tanmaykm for the commit! I cherry-picked your commit and made one additional commit ac95bb3 to fix some small issues. After these, I can reproduce correct code generations and everything works well.
This sounds like a good idea. Let's fix file names in this PR and leave the file content to a new one. |
Great! Looks good to me. |
It seems there is a small issue with generated julia function signature, when handling
To better reproduce this, I set up a MWE here
https://github.com/qiaojunfeng/test-julia-OpenAPI/tree/main
Since this is a julia-specific issue, I will ping technical committee member @tanmaykm. Thanks!
PR checklist
Commit all changed files.
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example
./bin/generate-samples.sh bin/configs/java*
.IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
master
(upcoming7.x.0
minor release - breaking changes with fallbacks),8.0.x
(breaking changes without fallbacks)