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

Missing some readme information #1020

Open
eloyesp opened this issue Sep 16, 2019 · 4 comments
Open

Missing some readme information #1020

eloyesp opened this issue Sep 16, 2019 · 4 comments
Projects

Comments

@eloyesp
Copy link

eloyesp commented Sep 16, 2019

I've heard about faraday long time ago even used it before, but the new website was not helpful to start working with the gem.

It took a lot of work to get something working and was after I've found the README file of my currently installed gem that I succeed.

I'm saying this not because I think the work on the new page made on #982 , it is just that there are some missing points that could make it better.

One of the issues I found is the lacking of full working example, the webpage tends to have partial configuration for each step, but it makes very difficult to understand how everything should go together.

An example like this, with most of the settings being used, would be nice.

connection = Faraday.new url: 'https://googleapis.com/v4/' do |faraday|
  faraday.authorization :Bearer, @token
  faraday.request :instrumentation
  faraday.response :logger
  faraday.response :raise_error
  faraday.response :json, :content_type => /\bjson$/
  faraday.adapter Faraday.default_adapter
end

connection.headers['Content-Type'] = 'application/json'

response = connection.post 'spreadsheets', '{"sample": 1}'

The adapter, for example, tends to be absent on the website.

Also, while it is a bit easier to make a one time read using different pages it makes it harder to use as reference document (searching for an attribute when writing code for example). May be an always visible table of contents, a search button and linking to actual documentation could help.

Regards, and thanks for all your hard work!

@iMacTia
Copy link
Member

iMacTia commented Sep 17, 2019

Thanks for the feedback @eloyesp, it is very much appreciated.
Point taken on the structure of the documentation, we'll discuss internally and come up with some improvements to help new developers getting started more easily

@technoweenie technoweenie added this to To Do in v1.0 Sep 19, 2019
@technoweenie
Copy link
Member

Hey @eloyesp, thanks for the feedback on the new website!

One consideration is that Faraday has a small core team with busy team members, with a community that isn't highly active. We want the documentation to serve the community, but it needs to be low maintenance. That's why we're using a basic GitHub Pages site.

Here are some things we can do:

  • Ensure the website docs link to the rubydocs. It has search, but of course includes a lot of internal code docs.
  • Add some kind of single page cheatsheet that includes a full Faraday example. I don't write much Ruby these days, so I'd actually really like this :)
  • Look into what can be done about the site nav. We're bound by rohanchandra/type-theme, but the maintainer has been receptive to PRs from us.

Does that sound like a good start?

@eloyesp
Copy link
Author

eloyesp commented Sep 20, 2019

I understand that time is always a problem, just hoped my feedback could help spending time on the right direction.

Linking to rubydocs seems like a great start, it could help move the details to in-code documentation that is easier to maintain.


The cheatsheet - full example sounds great, my recommendation in that point is showing of the ideal use case and implementation, not just a reference on what options are available, but also how it is expected you use it.

For example, given someone builds an adapter for twitter api using the following code, what changes would you recommend? Does using faraday like this makes any sense?

class TwitterAdapter
   CONN = Faraday.new 'https://api.twitter.com/' do |f|
    f.request :json
    f.request :logger # this should be added last
    f.adapter Faraday.default_adapter
  end

  def initialize token
    @connection = CONN.connection do |c|
       c.auth :Beacon, token
    end
  end

  def list
     get 'tweets'
  end
end

Regarding the site navigation I think that whenever possible I would make it a single page with sections instead of multiple pages, but that is just a personal preference.

Thanks.

@technoweenie
Copy link
Member

technoweenie commented Sep 20, 2019

For example, given someone builds an adapter for twitter api using the following code, what changes would you recommend?

It depends on the lifecycle of these objects. Is TwitterAdapter instantiated, used, and garbage collected per request? Or is it instantiated once, and used in multiple requests, possibly in different threads?

The easiest thing is to build the Faraday::Connection per request.

class TwitterAdapter
  def initialize(token)
    @connection = Faraday.new 'https://api.twitter.com/' do |f|
      f.token_auth token
      f.request :json
      f.response :logger
      f.adapter Faraday.default_adapter
    end
  end
end

But, this is inefficient, and can't take advantage of connection pools or keepalives (for the faraday adapters that support those). It looks like this is where you were leaning in your example, but it depends on modifying the Authorization header depending on the token. A global Faraday::Connection should not be modified once the first HTTP request has been made.

# BAD
conn = Faraday.new ...

conn.token_auth('ABC')
conn.get('/request/1')

conn.token_auth('DEF')
conn.get('/request/2')

# GOOD
conn = Faraday.new
conn.get('/request/1', nil, {'Authorization' => Faraday::Request::TokenAuthentication.header('abc')})
conn.get('/request/2', nil, {'Authorization' => %(Token token="def")})

With that in mind, I'd start off with an implementation like this:

class TwitterAdapter
   CONN = Faraday.new 'https://api.twitter.com/' do |f|
    f.request :json
    f.request :logger # this should be added last
    f.adapter Faraday.default_adapter
  end

  def initialize token
    @header = {
      'Authorization' => Faraday::Request::TokenAuthorization.header(token),
    }
  end

  def list
     get 'tweets'
  end

  private

  def get(path)
    res = CONN.get(path, nil, @header)
    res.body # json parsing, convert to response struct, etc
  end
end

A couple other advanced tips:

  1. All of the http verb methods support a block syntax, with as many of the method arguments as you like (they're all optional):

    conn.get '/request/3' do |req|
      req.params['a'] = 1
      req.headers['Authorization'] = %(Token token="abc")
    end
  2. If you find yourself trying to do something like conn.send(http_verb, path, ...), then you should use #run_request instead:

    # these args are mandatory for some reason
    conn.run_request :get, '/request/4', nil, nil do |req|
      req.params['a'] = 1
      req.headers['Authorization'] = %(Token token="abc")
    end

EDIT: If it helps at all, the sample requests above were actually made to this request bin.

@technoweenie technoweenie moved this from To Do to Backlog in v1.0 Oct 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
v1.0
Backlog
Development

No branches or pull requests

3 participants