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

[proposal] HTTPipe.Conn.new/1 function #9

Open
stephenmoloney opened this issue Feb 20, 2017 · 3 comments
Open

[proposal] HTTPipe.Conn.new/1 function #9

stephenmoloney opened this issue Feb 20, 2017 · 3 comments

Comments

@stephenmoloney
Copy link

stephenmoloney commented Feb 20, 2017

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 stephenmoloney changed the title [proposal] HTTPipe.conn.new/1 function [proposal] HTTPipe.Conn.new/1 function Feb 20, 2017
@DavidAntaramian
Copy link
Owner

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.

@stephenmoloney
Copy link
Author

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
Copy link
Author

stephenmoloney commented Feb 24, 2017

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants