[Ruby] add support for "faraday" by dkliban · Pull Request #3405 · OpenAPITools/openapi-generator

dkliban · 2019-07-20T11:49:46Z

Description of the PR

This patch adds ability to specify which library to use when generating a Ruby Client. The two options are 'typhoeus' and 'faraday'. The implementation with 'typhoeus' was already present and this patch adds support for 'faraday'.

I have not tested this with the SSL option or token Auth.

I did test that the client can use basic Auth and perform both JSON and multipart/form-encoded requests.

dkliban · 2019-07-20T14:59:19Z

modules/openapi-generator/src/main/resources/ruby-client/faraday_api_client.mustache

This template is very similar to api_client.mastache. The main difference is the implementation of call_api() and build_request(). Would it make sense to combine the two templates and simply pick the implementation of these methods based on the library selected?

This file is ~400 lines. Keeping both templates in sync when changes happen outside those methods will be a PITA I think.
So I'd say for long term maintainability, it makes sense to merge them but I'm an outsider of this project, I hope maintainers can chime in.

The generator supports a library option, which allows library specific overrides for individual files. Using this may make the logic and template separation clearer. The Java client generator is a good example. Let us know if you'd like to move toward that option.

Yes, I think there's a library option added by this PR:

https://github.com/OpenAPITools/openapi-generator/pull/3405/files#diff-23d62a3ed9c41293f930468ed3ee74a8R27

so the other 2 options (Ruby HTTP library names) are not needed

This template is very similar to api_client.mastache

I'll see if I can combine the two ...

Yes, I think there's a library option added by this PR:

https://github.com/OpenAPITools/openapi-generator/pull/3405/files#diff-23d62a3ed9c41293f930468ed3ee74a8R27

so the other 2 options (Ruby HTTP library names) are not needed

Makes sense. I'll remove them.

jcarres-mdsol · 2019-07-22T17:31:51Z

modules/openapi-generator/src/main/resources/ruby-client/faraday_api_client.mustache

@dkliban Thanks so much for this PR!
I wonder if you have considered how to add middlewares here to fully use the power of Faraday. One could add caching or tracing or whatever in here. I think that's one of the best things about moving to Faraday

jcarres-mdsol · 2019-07-22T17:36:09Z

modules/openapi-generator/src/main/resources/ruby-client/faraday_api_client.mustache

I'm torn about making all failign cases the same Exception class, it may be difficult to distinguish among errors if that's what the caller wants to do. Also here you probably want to deal with Net::ReadTimeout and Faraday::ConnectionFailed

@meganemura

Solution: add support for Faraday library This patch builds on the work started by @meganemura

Solution: switch ruby client to use Faraday This patch relies on a forked version of the openapi-generator-cli. Once the PR that introduces this functionality is merged into openapi-generator, the ruby client generation should be switched back to using the docker container for openapi-generator. OpenAPITools/openapi-generator#3405 re: #4896 https://pulp.plan.io/issues/4896

Solution: switch ruby client to use Faraday This patch relies on a forked version of the openapi-generator-cli. Once the PR that introduces this functionality is merged into openapi-generator, the ruby client generation should be switched back to using the docker container for openapi-generator. OpenAPITools/openapi-generator#3405 Required PR: pulp/pulpcore#226 re: #4896 https://pulp.plan.io/issues/4896

...es/openapi-generator/src/main/java/org/openapitools/codegen/languages/RubyClientCodegen.java

wing328 · 2019-07-24T13:27:55Z

...es/openapi-generator/src/main/java/org/openapitools/codegen/languages/RubyClientCodegen.java

+        CliOption libraryOption = new CliOption(CodegenConstants.LIBRARY, "HTTP library template (sub-template) to use");
+        libraryOption.setEnum(supportedLibraries);
+        // set FARADAY as the default
+        libraryOption.setDefault(FARADAY);


What about using Faraday as the default later (say 4.2.0) after we collect more feedback from the users?

That would be great.

...es/openapi-generator/src/main/java/org/openapitools/codegen/languages/RubyClientCodegen.java

Solution: Make Typhoeus default This patch also updates the api_client template for Faraday to include query params with requests.

wing328 · 2019-07-30T13:56:14Z

bin/ruby-petstore-faraday.json

@@ -0,0 +1,5 @@
+{
+  "gemName": "petstore",


I think typhoeus is the default so we need to add "library": "faraday", here if I'm not mistaken.

I'll take care of it in another PR.

wing328 · 2019-08-01T07:43:59Z

I've filed #3520 for various improvements

wing328 and others added 8 commits July 19, 2019 14:15

WIP: add faraday support to Ruby client

d79ee59

update samples

52432fb

bin/console

5bc2ce4

https only

a60625f

Add a test for ruby-faraday client options

8e1b157

Implement faraday client

7efeb78

ruby-faraday: Add a gemspec template for faraday

47122f7

Remove Gemfile.lock from ruby-client-faraday

c6f323d

dkliban commented Jul 20, 2019

View reviewed changes

jcarres-mdsol reviewed Jul 22, 2019

View reviewed changes

Problem: can't use Faraday library for ruby clients

585d5c9

Solution: add support for Faraday library This patch builds on the work started by @meganemura

dkliban force-pushed the ruby-client-faraday branch from d13f4de to 585d5c9 Compare July 23, 2019 14:57

dkliban mentioned this pull request Jul 23, 2019

Problem: ruby client can't form-encode PUT requests pulp/pulp-openapi-generator#21

Merged

wing328 reviewed Jul 24, 2019

View reviewed changes

...es/openapi-generator/src/main/java/org/openapitools/codegen/languages/RubyClientCodegen.java Outdated Show resolved Hide resolved

wing328 reviewed Jul 24, 2019

View reviewed changes

...es/openapi-generator/src/main/java/org/openapitools/codegen/languages/RubyClientCodegen.java Show resolved Hide resolved

wing328 reviewed Jul 24, 2019

View reviewed changes

dkliban commented Jul 24, 2019

View reviewed changes

...es/openapi-generator/src/main/java/org/openapitools/codegen/languages/RubyClientCodegen.java Outdated Show resolved Hide resolved

Problem: Faraday is set as default library

b708032

Solution: Make Typhoeus default This patch also updates the api_client template for Faraday to include query params with requests.

dkliban force-pushed the ruby-client-faraday branch from 4006ade to b708032 Compare July 25, 2019 12:46

wing328 reviewed Jul 30, 2019

View reviewed changes

wing328 added the Client: Ruby label Jul 31, 2019

wing328 added the Enhancement: Feature label Jul 31, 2019

wing328 added this to the 4.1.0 milestone Jul 31, 2019

wing328 merged commit bcc3a9e into OpenAPITools:master Aug 1, 2019

wing328 changed the title ~~Ruby client faraday~~ [Ruby] add support for "faraday" Aug 1, 2019

This was referenced Aug 1, 2019

WIP: Ruby faraday client #1542

Closed

[Ruby] Add support for Faraday (HTTP lib) #3270 #1422

Closed

johngallagher mentioned this pull request Nov 13, 2019

[REQ] [RUBY] [FARADAY] Allow middleware to be configured #4476

Closed

tsuwatch mentioned this pull request Oct 13, 2020

[BUG][Ruby] undefined method in api_client.rb #7708

Closed

6 tasks

Uh oh!

Conversation

dkliban commented Jul 20, 2019

Description of the PR

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!

dkliban Jul 24, 2019 • 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!

jcarres-mdsol Jul 22, 2019 • 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!

jcarres-mdsol Jul 22, 2019 • 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!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wing328 commented Aug 1, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

dkliban Jul 24, 2019 •

edited

Loading

jcarres-mdsol Jul 22, 2019 •

edited

Loading

jcarres-mdsol Jul 22, 2019 •

edited

Loading