Event system + activity logging

[ixti]

Proposed API:

# Add default (global) `:connect` event listener 
HTTP.on(:connect) { ... }

# Add `:request` event listener for particular client instance
client = HTTP.auth("Bearer 123")
client.on(:request) { ... }

With proper architecture, IMO it will become possible to extract timeouts out of connection/client completely.

[zanker]

Timeouts requires you have direct access to socket write and reads. You can't do that with an API.

I'm not sure I see an advantage to this. Celluloid already lets you do the async bits. Seems like unneeded abstraction.

Sent from my iPhone

On Apr 12, 2015, at 16:32, Alexey Zapparov notifications@github.com wrote:

Proposed API:

Add default (global) :connect event listener

HTTP.on(:connect) { ... }

Add :request event listener for particular client instance

client = HTTP.auth("Bearer 123")
client.on(:request) { ... }
With proper architecture, IMO it will become possible to extract timeouts out of connection/client completely.

—
Reply to this email directly or view it on GitHub.

[tarcieri]

@zanker this kind of API can be useful for people writing things like testing frameworks who want to instrument clients for various purposes. Right now they're using monkeypatches.

See this discussion:

https://groups.google.com/forum/#!topic/httprb/ISln457aPUc

[ixti]

What comes to VCR support (from ML) actually that can't be done without monkey patching anyway :D And it's already done (webmock backend for VCR). I was mostly thinking about kind of a way to "monitor" progress. Like log requests and timings, probably sending "halt" to client on some events...

[tarcieri]

It'd be nice to have something like Reel::Spy in http.rb:

https://github.com/celluloid/reel/wiki/Spy

A system like this would make it easy to implement.

[ixti]

Proposed API:

module HTTP
  def self.spy(spy = Spy, &block)
    case
    when spy.is_a?(Spy)
      branch(:spy => spy)
    when spy.is_a?(Class) && spy.ancestors.include?(Spy)
      branch :spy => spy.new(&block)
    else
      fail "Instance or subclass of Spy expected"
    end
  end
end

client_with_spy = HTTP.spy do
  on(:connect) { |client| ... }
  on(:request) { |client| ... }
end

[pezra]

Would either of the proposed APsI allow modification of the request flow. Ie, could an on(:request) bypass the actual HTTP request in favor of a cached response instead? Allowing a "spy" to do that seems a little unintuitive but maybe not out of the question.

[tarcieri]

@pezra it could. It doesn't have to be called "spy"

[ixti]

@tarcieri if we are talking about events, then I don't see how it can modify workflow. I mean I would like to avoid (and I know you too) middleware alike architecture.

[Senjai]

RE: logging, I probably wouldn't complicate it too much. I think hooks as extension points would be great, but they're not required to make logging better. Couldn't we just add a debug option on configuration and a logger class? If we choose to add hooks for extension, the logger could be registered as a default hook if the option is true.

Until that time, I think these could be two separate issues. Any opinions?

[Senjai]

@pezra I think hooks should be different from composition. Hooks should allow you to execute code when a thing occurs. But it should not block other code from running.

Instead I think it'd be nice to have a connection class, or request class or whatever that can be substituted in favor of a dummy request object via composition if you required that kind of behavior.

[pezra]

@ixti, I agree that allowing an event handler to modify the flow send awkward.

@Senjai, an option specify the connection class to use might be workable. Wild such a PR be accepted?

[tarcieri]

@Senjai we already have HTTP::Connection and HTTP::Request classes

@pezra if it's not too invasive and doesn't involve weird delegators/proxy objects

[ixti]

Resolved by #499