httpipe icon indicating copy to clipboard operation
httpipe copied to clipboard

[proposal] HTTPipe.Conn.new/1 function

Open stephenmoloney opened this issue 8 years ago • 3 comments

It would be nice if there was a function like this:

HTTPipe.Conn.new(%HTTPipe.Request{})

Example of how I am doing it now:

    req = %HTTPipe.Request{
      method: :post,
      url: "/cloud/project/#{service_name}/user/#{user_id}/regeneratePassword"
    }
    Map.put(HTTPipe.Conn.new(), :request, req)

Would be nice if I could

%HTTPipe.Request{
      method: :post,
      url: "/cloud/project/#{service_name}/user/#{user_id}/regeneratePassword"
    }
|> HTTPipe.Conn.new()

stephenmoloney avatar Feb 20 '17 00:02 stephenmoloney

Just letting you know I definitely saw this, and I'm thinking about it. I've heard from someone else that perhaps the Conn struct should be a reusable element for multiple reasons:

  1. The Conn could be responsible for maintaining a pool identifier
  2. HTTP 2 will not have the same request/response cycle, so to make it version agnostic, Conn may just represent a generic connection and could operate with requests, responses, and with any ordering of the two.

DavidAntaramian avatar Feb 24 '17 04:02 DavidAntaramian

About those items mentioned, I haven't thought about it too much.

  • About the Conn struct being responsible for maintaining a pool identifier - i'm not sure this is a strong enough reason to make this a reusable element. Instead, the Conn.t could have a pool field which the user has to set. Then the HTTPConn.execute function will be responsible for getting it executed by the correct pool. Does that make sense?

About HTTP2, I'd need to do more research before I could say for sure. Some modifications are probably need to HTTPipe.Conn.t and Request.t to make this work. But I'm still not convinced that it demands a reusable Conn.t but I'd have to dwell on that.

  • HTTP2 - could this be as simple as: (??)
    1. removing http_version from HTTPipe.Request.t and putting it into HTTPipe.Conn.t instead.
    2. making the request: field into a list of requests
    3. making the response: field into a list of responses
    4. could make the Conn.t to :status_executed only when all request responses returned

One thing I did think though was if genstage and streaming is being introduced, perhaps a rethink would be needed then or maybe a new type.

stephenmoloney avatar Feb 24 '17 09:02 stephenmoloney

Another way might be to create abstractions at a higher level than HTTP.Conn.t - say %HTTP2{ conns: [%HTTP.Conn{},%HTTP.Conn{}], pool: _ }

stephenmoloney avatar Feb 24 '17 09:02 stephenmoloney