[New Generator] Rust API client generator by farcaller · Pull Request #6092 · swagger-api/swagger-codegen

farcaller · 2017-07-17T15:36:25Z

PR checklist

Read the contribution guidelines.
Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
Filed the PR against the correct branch: master for non-breaking changes and 3.0.0 branch for breaking (non-backward compatible) changes.

Description of the PR

Tracking PR for Rust client implementation.

The first commit adds a subset of apis requested in #5905.

farcaller · 2017-07-17T15:37:05Z

I'll add a few comments on the specifics inline

farcaller · 2017-07-17T15:37:12Z

samples/client/petstore/rust/Cargo.toml

@@ -0,0 +1,17 @@
+[package]
+name = "petstore-client"


these settings would be exposed as flags.

farcaller · 2017-07-17T15:37:40Z

samples/client/petstore/rust/examples/client.rs

@@ -0,0 +1,44 @@
+extern crate petstore_client;


a sample file, not to be part of generated api

farcaller · 2017-07-17T15:38:32Z

samples/client/petstore/rust/src/apis/add_pet_api.rs

+use super::models;
+use super::Error;
+
+pub fn add_pet<C: hyper::client::Connect>(


rust naming guidelines dictate snake_case in function names

farcaller · 2017-07-17T15:39:34Z

samples/client/petstore/rust/src/apis/add_pet_api.rs

+use super::Error;
+
+pub fn add_pet<C: hyper::client::Connect>(
+    prefix: &str,


I'm not sure about what's the common pattern for api use, really. This sample client will require two things: a http (or https) prefix as a string and the hyper.rs Client (the web client) plus any arguments.

farcaller · 2017-07-17T15:40:01Z

samples/client/petstore/rust/src/apis/add_pet_api.rs

+    let mut req = hyper::Request::new(
+        hyper::Method::Post,
+        format!("{}/pet", prefix).parse().unwrap());
+    let serialized = serde_json::to_string(pet).unwrap();


no error handling for brevity of the example.

farcaller · 2017-07-17T15:41:37Z

samples/client/petstore/rust/src/apis/add_pet_api.rs

+    Box::new(
+        cli.request(req).and_then(|res| { res.body().concat2() })
+        .map_err(|e| Error::from(e))
+        .and_then(|_| futures::future::ok(()))


the api doesn't dictate the return value so nothing is returned even though the sample server returns a pet instance.

npm server choked up on {"id":1337,"category":null,"name":"barker","photoUrls":[],"tags":[],"status":null} here, http://petstore.swagger.io worked fine.

farcaller · 2017-07-17T15:42:20Z

samples/client/petstore/rust/src/apis/mod.rs

+
+use super::models;
+
+mod get_pet_by_id_api;


there's a single namespace, so modules (and files) would have to be named differently from the actual functions.

farcaller · 2017-07-17T15:43:11Z

samples/client/petstore/rust/src/models/pet.rs

+    id: Option<i64>,
+    category: Option<super::Category>,
+    name: String,
+    #[serde(rename = "photoUrls")] photo_urls: Vec<String>,


again, snake case here, and renamed fields need an annotation.

wing328 · 2017-07-17T15:44:50Z

@farcaller thanks for the Rust Petstore sample, I'll start the reverse engineering process tomorrow and let you know if I've any question.

farcaller · 2017-07-18T07:34:56Z

I poked more around the go generated code and brought the example to the similar level of api. The changes, basically, are that there's an APIClient now, that incapsulates access to the API, and a dedicated pet_api.rs that's supposedly implements all the Pet apis. This extension fully builds on the previous example, though, so it would be fine if the generator produced the functional API as in original commit, and I can tackle the object API later myself when I'll have a better understanding of swagger internals.

wing328 · 2017-07-18T14:36:21Z

Quick question: would it be possible to group add_pet_api.rs, get_pet_by_id_api.rs, update_pet_with_form_api.rs into a single rs file (e.g. pet_api_func.rs)?

farcaller · 2017-07-18T15:18:27Z

Absolutely. I'll update the PR

wing328 · 2017-07-18T16:18:12Z

@farcaller I've submitted the initial version via #6105

Please comment on the code in that PR and we'll fix the remaining issues one by one.

lucab · 2017-07-19T10:00:12Z

samples/client/petstore/rust/Cargo.toml

+base64 = "*"
+futures = "*"
+hyper = "*"
+url = "*"


Would you mind annotating which dependency version is the codegen targeting?

yep, as soon as I figure those myself :-) so far the generated code doesn't compile

lucab · 2017-07-19T11:47:51Z

samples/client/petstore/rust/src/apis/get_pet_by_id_api.rs

+    prefix: &str,
+    cli: &hyper::client::Client<C>,
+    pet_id: i64,
+    ) -> Box<Future<Item = models::Pet, Error = Error>> {


(Here and in other generated methods)
Would you consider returning a more complex type? The Item here is losing any additional information about the response (header, status, etc) which I've found useful in some situations (e.g. token authenticated endpoints redirecting to token provider). For reference, reqwest does this with a richer Response struct + trait.

For other generators (e.g. PHP, Ruby), there's a corresponding "WithHttpInfo" method (e.g. getPetByIdWithHttpInfo) to return a list with the response (if any), HTTP status code and HTTP headers.

Not sure if Rust supports returning a list. If not, we will need to create an ApiResponse object with response, status and headers as properties.

For Rust, we can follow similar approach by creating a corresponding WithHttpInfo method or we can default the method (e.g. getPetById) to return the response together with status code and headers.

I think we can return response object along with the parsed body. Let's focus on making the current implementation compileable and then we can improve.

It's also worth sharing other API clients usually come with a corresponding "Async" method (e.g. addPetAsync) for async function call. Not sure how that should be done in Rust but worth thinking ahead with this future requirement.

All this code is async-only. I don't think that it makes sense to have a matching sync version.

Ah ok. Good to know. One less thing to consider then.

wing328 · 2017-07-21T13:58:28Z

Merging this (Rust Petstore sample) into master first similar to what we've done for PowerShell and other generators that follow similar approach so as to give credits back to the contributor.

For further discussion on the Rust generator, please continue in #6105, which is work-in-progress.

Added rust client example

5761118

farcaller commented Jul 17, 2017

View reviewed changes

farcaller mentioned this pull request Jul 17, 2017

[New Generator] Rust API client generator #5905

Closed

wing328 added Client: Rust Enhancement: New generator labels Jul 17, 2017

wing328 added this to the v2.3.0 milestone Jul 17, 2017

Added a proper apiclient / configuration based example

d8441f1

wing328 mentioned this pull request Jul 18, 2017

[Rust] add new Rust client generator #6105

Merged

3 tasks

lucab reviewed Jul 19, 2017

View reviewed changes

wing328 merged commit 1f133e8 into swagger-api:master Jul 21, 2017


		use super::models;

		mod get_pet_by_id_api;

Conversation

farcaller commented Jul 17, 2017

PR checklist

Description of the PR

Uh oh!

farcaller commented Jul 17, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wing328 commented Jul 17, 2017

Uh oh!

farcaller commented Jul 18, 2017

Uh oh!

wing328 commented Jul 18, 2017

Uh oh!

farcaller commented Jul 18, 2017

Uh oh!

wing328 commented Jul 18, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lucab Jul 19, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wing328 commented Jul 21, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lucab Jul 19, 2017 •

edited

Loading