[Proposal] Allow adapter specific data to be stored inside Tesla.Env

[amatalai]

Example:

#little change to adapter
defmodule Tesla.Adapter.Hackney do
    def call(env, opts) do
      with  {:ok, status, headers, body, location} <- request(env, opts || []) do
        %{env | status:   status,
                headers:  headers,
                body:     body,
                private: %{final_location: location}}
      end
    end

    defp handle({:ok, status, headers, ref}) do
      with  location <> "" <- :hackney.location(ref),
               {:ok, body} <- :hackney.body(ref) do
        {:ok, status, headers, body, location}
      end
    end
  end

#usage
iex(1)> Tesla.get("https://github.com/teamon/tesla", opts: [follow_redirect: true])
%Tesla.Env{
  url: "https://github.com/teamon/tesla",
  private: %{final_location:  "https://github.com/teamon/tesla"}
}

iex(2)> Tesla.get("https://goo.gl/8hfJ7F", opts: [follow_redirect: true])
%Tesla.Env{
  url: "https://goo.gl/8hfJ7F",
  private: %{final_location:  "https://github.com/teamon/tesla"}
}

[teamon]

While I do get the idea you could use `Tesla.put_opt(:location, ...)` and then access the value with `env.opts.location`. Is there a reason why opts might not be a good solution?

…

On Wed, 25 Jan 2017 at 01:38, Tobiasz Małecki ***@***.***> wrote: Example: #little change to adapter defmodule Tesla.Adapter.Hackney do def call(env, opts) do with {:ok, status, headers, body, location} <- request(env, opts || []) do %{env | status: status, headers: headers, body: body, private: %{final_location: location}} end end defp handle({:ok, status, headers, ref}) do with location <> "" <- :hackney.location(ref), {:ok, body} <- :hackney.body(ref) do {:ok, status, headers, body, location} end end end #usage iex(1)> Tesla.get("https://github.com/teamon/tesla", opts: [follow_redirect: true]) %Tesla.Env{ url: "https://github.com/teamon/tesla", private: %{final_location: "https://github.com/teamon/tesla"} } iex(2)> Tesla.get("https://goo.gl/8hfJ7F", opts: [follow_redirect: true]) %Tesla.Env{ url: "https://goo.gl/8hfJ7F", private: %{final_location: "https://github.com/teamon/tesla"} } — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#48>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAfk8tV-nq-xqoqvo6hb-R7Vmlb66e3ks5rVplqgaJpZM4Ls93b> .

[amatalai]

Sure, opts could be used, but I think it's would be cleaner to not mix different entities under same key.

For me :opts is write-only for user and read-only for Tesla when :private will be opposite.
What do I mean by this?

When you do Tesla.get(url, opts: [something: true]) you probably don't care about what's saved under :opts key in response, because you know what you have passed in function call. On the other hand Tesla shouldn't use data from :private when making request, because this field would allow to store inconsistent data(specific to one adapter/middleware), which from end user perspective may be useful.

#some pseudocode to make this 100% clear

important_in_request = [follow_redirect: true]
ignored_by_tesla = [random_key: :value]

%Tesla.Env{
   opts: _user_probably_dont_care,
   private: additional_data_that_user_may_be_interested_in
} = Tesla.get(url, opts: important_in_request, private: ignored_by_tesla)

[teamon]

I agree mixing opts with private might not be a good idea.

One question remains - what should be the desired adapter's behaviour and rules for using private properties? (Assuming client code does not modify adapters)

In case of "follow redirects" I think this should be a tesla middleware like this

[amatalai]

My first idea for private properties can be described as "allow to store anything useful, don't merge to master anything that tries to read from it". I'll think about it more later, but unfortunately I'm little busy at the moment.

about FollowRedirects middleware
I like this, it will allow me to remove custom implementation from my library
one note: httpc default for autoredirect is true, you should probably disable it inside this middleware

btw I made middleware for data compression
https://github.com/amatalai/link_preview/blob/1.0.0/lib/compression_middleware.ex
It's work in progress now, but I think that I'll have time to finish it in next few days (I'll make separate issue tomorrow to discuss details)

[teamon]

@amatalai sooooo, what are we doing with this? ;)

[amatalai]

I wish I could create PR with some proposal to push work on this forward, but lately I don't have time to do that.

[amatalai]

I need to revisit this idea. With ability to add middleware before adapter it should be easy to save raw headers etc with simple middleware and without any breaking changes.

[teamon]

As mentioned in #104 there was always a possibility to add something right around adapter :)

[teamon]

Please see possible solution in #37

[amatalai]

Yeah, this wouldn't be useful at the moment