artifice
artifice copied to clipboard
Limit activation to a specific host/port combination
The modifications in these change sets provide the ability to limit activation to a specific host/port combination instead of universally overriding Net::HTTP.
I've implemented this because of a specific need that I have in production where an app was developed as an API endpoint and a "web" endpoint intended to run on different ports, but performance considerations have forced us to push them into a single application, so I can use artifice to still use the separate API without requiring network overhead, yet I will still get correct behaviour from the APIs that I must still reach over the network.
I've considered the merits of this as well. Ultimately, I've found it so convenient to handle things like that inside of the endpoint I create that I've come to see little need for such a feature.
I think the motivation is sensible enough, but ultimate (to me) this feature would add unnecessary complexity as part of the core library.
Have you thought about making this an extension?
I don't know if this is helpful, but an extension exists that enables you to allow a real http request to slip through your endpoint. https://github.com/remi/artifice-passthru
That's my $0.02
artifice-passthru is useful and essentially does the same as #activate_for, but it's also a bit late for my production use case. What I don't like about aritifice-passthru is threefold:
- It's sort of the inverse of what I'm doing here, at least in the examples given.
- It's substantially more complex than what I'm doing here.
- It's fragile. The changes that I've made are much less fragile because they completely fall back to the standard behaviour unless the endpoint is matched.
The complexity and fragility are big arguments against artifice-passthru and the "extension" approach, IMO. Compared to what I've provided, artifice-passthru has to keep a thread-local variable on a per-request basis to rescue the case when the block calls #passthru!, and it takes a perfectly good Net::HTTPResponse and turns it into a Rack response just so that it can be turned into another Net::HTTPResponse. Outbound it also creates a new HTTPRequest rather than forcing 'super' behaviour on the existing request. Those new objects and transformations add a pretty hefty overhead that isn't necessary.
The changes I've done to add #host/#port to the #endpoint tracking appear to me to be in the spirit of what Artifice is doing (single-level artifice override), and would be fairly easy to extend if core-Artifice was modified to support either stacked Artifice endpoints or multiple Artifice endpoints.
I'll admit that this may not be the best way to implement this behaviour, but I think that something a little more invasive to the core is probably the safest and fastest way to let Artifice work with both real and fake calls, and I've tried very hard to make the minimal number of changes. (The biggest change I could probably dump is the #use_rack_endpoint? method, as Net::HTTP uses the @newimpl variable for much the same sort of dispatch change in-line.)
That's fine. Either way, perhaps this would be more appropriate as a plugin?
If it's especially painful to write extensions for artifice, perhaps we should focus on fixing that? I like the idea of keeping the core as simple as possible and focusing on providing robust extensibility if that's what's needed.
Thoughts?
OK, let's talk plug-in capability and efficiency. The core piece of functionality missing from Artifice is the ability to do an efficient dispatch to something other than the singular Rack endpoint provided on activation. This affects different users in different ways. Some users want the dispatch to be to multiple Rack endpoints; I want (and the person who wrote artifice-passthru wants) the dispatch to be to a real Net::HTTP connection.
-
I don't have a particular need for multiple endpoint dispatch, but I can imagine mechanisms to return the endpoint required for a particular request; it would need to be a modification such that
rack_request = RackRequest.new(self.class.endpoint)
doesn't necessarily callself.class.endpoint
, but an extension point. Perhaps a method#endpoint
would be the way to make this pluggable.def endpoint self.class.endpoint end
-
On the other hand, the mechanism that I require needs something just a little more invasive. The only way to efficiently do this is pretty much what I've done: a modification to
#connect
and#request
that will call the parent class method (throughsuper
) if the connection is not to be handled by the Rack endpoint but the standard Net::HTTP mechanisms. This, too, could at least partially be handled by the#endpoint
method, but I don't believe that this is the best choice for this.
I can try to work this up as a different patch set if you're interested.