[Rust] Split out request logic, implement form parameters#528
Merged
wing328 merged 3 commits intoOpenAPITools:masterfrom Jul 23, 2018
Merged
[Rust] Split out request logic, implement form parameters#528wing328 merged 3 commits intoOpenAPITools:masterfrom
wing328 merged 3 commits intoOpenAPITools:masterfrom
Conversation
Contributor
Author
|
CI failure is a network or mvn flake it looks like |
euank
force-pushed
the
rust-split-out-request-making
branch
from
92c8dd4 to
ea41745
Compare
bjgill
reviewed
Jul 19, 2018
Contributor
bjgill
left a comment
bjgill
left a comment
There was a problem hiding this comment.
I really like what you've done - it makes the mustache templates a lot more readable. I might need to move rust-server over to use this pattern as well.
Contributor
There was a problem hiding this comment.
Should these be pub(crate)?
Contributor
Author
There was a problem hiding this comment.
Yup, updated, thanks!
Contributor
There was a problem hiding this comment.
Is it necessary to have multiple copies of this file?
Contributor
Author
There was a problem hiding this comment.
Oops, this was a rebase mess-up to have both here; this one's gone now.
Contributor
There was a problem hiding this comment.
We only ever call this with val=true. Do we need to have val?
Contributor
There was a problem hiding this comment.
I've not reviewed this in detail - I assume you've just copied the logic across.
Contributor
Author
There was a problem hiding this comment.
More or less.
I did add minor error handling around uri parsing that was an unwrap before and restructured it a bit to make it read cleaner, but it's meant to be just a straight copy+paste.
Other than forms. I did add form parameters since those weren't present at all before.
This reduces the number of variables which are used in the generated operations, thus fixing OpenAPITools#512. This also fixed a TODO related to URI parsing errors. Other than that, it is meant to be functionally identical.
Up until now, they just weren't there at all
euank
force-pushed
the
rust-split-out-request-making
branch
from
ea445b6 to
546f510
Compare
bjgill
approved these changes
Jul 23, 2018
Member
bcourtine
added a commit
to bcourtine/openapi-generator
that referenced
this pull request
Oct 17, 2018
bjgill
pushed a commit
that referenced
this pull request
Oct 26, 2018
* Port of PR swagger-api/swagger-codegen#8804. * Correction of conflict with PR #528 (missing template file). * Add rust-reqwest samples to Circle CI tests. * Add integration test pom.xml file with launcher to trigger cargo execution. * Deduplicate Maven project name. * Fix "api_key" header for Petstore. * Better API key management. * Fix query param for lists of objects other than strings (numbers, etc.). * Update to reqwest 0.9, and refactor of header management (using reqwest transition feature). * Merge scripts generating rust-hyper and rust-reqwest samples. * Consistent full stops. * Use raw variables in all Rust mustache templates. * Replace production build in CI with a quick simple check. * Update samples. * Finish Reqwest 0.9 migration (removing "hyper 0.11" transition feature). * Configuration implements Default trait. * API template reorganized: HashMap is not required anymore. * Revert "Merge scripts generating rust-hyper and rust-reqwest samples." This reverts commit 970f996. * Remove deprecated "-XX:MaxPermSize" java arg.
A-Joshi
pushed a commit
to ihsmarkitoss/openapi-generator
that referenced
this pull request
Feb 27, 2019
…ls#528) * [Rust] Move request logic into standalone file This reduces the number of variables which are used in the generated operations, thus fixing OpenAPITools#512. This also fixed a TODO related to URI parsing errors. Other than that, it is meant to be functionally identical. * [Rust] Add support for non-file form params Up until now, they just weren't there at all * [Rust] Use more rustic terms in example
A-Joshi
pushed a commit
to ihsmarkitoss/openapi-generator
that referenced
this pull request
Feb 27, 2019
…1258) * Port of PR swagger-api/swagger-codegen#8804. * Correction of conflict with PR OpenAPITools#528 (missing template file). * Add rust-reqwest samples to Circle CI tests. * Add integration test pom.xml file with launcher to trigger cargo execution. * Deduplicate Maven project name. * Fix "api_key" header for Petstore. * Better API key management. * Fix query param for lists of objects other than strings (numbers, etc.). * Update to reqwest 0.9, and refactor of header management (using reqwest transition feature). * Merge scripts generating rust-hyper and rust-reqwest samples. * Consistent full stops. * Use raw variables in all Rust mustache templates. * Replace production build in CI with a quick simple check. * Update samples. * Finish Reqwest 0.9 migration (removing "hyper 0.11" transition feature). * Configuration implements Default trait. * API template reorganized: HashMap is not required anymore. * Revert "Merge scripts generating rust-hyper and rust-reqwest samples." This reverts commit 970f996. * Remove deprecated "-XX:MaxPermSize" java arg.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR checklist
./bin/to update Petstore sample so that CIs can verify the change.cc @frol @farcaller @bjgill
Description of the PR
This PR contains two changes which I needed to make for a particular swagger client to work.
The first moves most request logic into a standalone file / method. This makes it much erasier to avoid variable conflicts since the
Request.executemethod doesn't have the generatedparams in its scope, and as such doesn't have to worry about conflicting with them.I intend for
.executeto behave identically to what we previously generated per operation.I did, however, also implement form data while I was at it. That is included as a second separate commit, and I can file it separately if that's preferred.
Fixes #512, #525