"Connection" should be fully wrapped with mocks #1418

Open
tokengeek opened this Issue Dec 24, 2012 · 6 comments

Projects

None yet

2 participants

@tokengeek
fog member

Whilst working on #1392 and general other changes I got the feeling that we are missing a layer or two of abstraction.

We have Fog::Connection which is a wrapper to Excon. However we get back an Excon::Response object which is technically an implementation detail (which HTTP library we are using).

I think we should be returning a fog specific response. This would also allow us to move the response body parsing to this response object as well.

If you look within a Fog::Service however, it's collections have requests and mocked requests. Where this is standing out is for the local virtualisation systems like VirtualBox and Vsphere.

The connection within those services is not a HTTP connection to a remote service. Often a 'connection' through a native Ruby library to the hypervisor.

The other abstraction problem is fog's mocking is implemented at the Service level. So the factory will give you one of two different classes Real or Mock. So we are duplicating code or relying on Shared modules or worse having two different implementations.

So rather than just isolating the live connection to an API we actually have two blocks of code that rely on a HTTP interface.

So we are leaking Excon out and we are setting ourselves up so that the request/response model through it's interface is the definitive way.

My idea for improving this so far:

  • Create errors and a response object as the response for a "request"
  • Providers update request to return response object
  • Response object returns parsed data not "body" for parsing
  • Abstract response frees providers to create their own Connection subclasses, different input standard output (even if still provider specific data).
  • Providers raise suitable general errors for any HTTP or code/socket problems with messages
  • Move mocking from the Service level down to the Connection level

Not exactly trivial changes. Several possible strategies to phase in however.

@geemus
fog member

@tokengeek - good write up. I definitely have thought that moving the mocks from service level to connection would be great (ie using excon stubs for http case). Massive rewrite involved there, but does clean things up. I also can certainly see the value of abstracting further away from excon, the closeness is at least in part because excon was originally part of fog (and I later extracted it). Also partly that we used to only have http interfaces. Anyway, definitely some good points here that would be worth exploring.

@geemus
fog member

@tokengeek - One thing I would say is that we should probably try to break this apart into some smaller steps and document conversion process and expectations to make it easier to coordinate.

@tokengeek
fog member

Yes. There's a few tickets worth of stuff here and it does need upgrade documentation for others to follow.

I'm wondering if parts of this can be done in parallel with existing code and structure.

So Service::Provider.new could either return a "Real", a "Mock" or the new variant with the connection mockable. Both would honour Fog.mocking?

That would mean two possible ways of mocking but they would be documented as the new/old way and a provider should not do both.

@geemus
fog member

@tokengeek - that sounds like a pretty reasonable approach. My only concern there would be that it would make it harder to know the codebase and maintain across providers, but increasingly that is not the case (I'm one of the only people that realistically jumps between code bases). Otherwise it seems good and to end users it shouldn't really matter which approach is supported.

@tokengeek
fog member

@geemus - I think it would be acceptable short term to allow us to transition. The alternative is trying to get everything changed in one go which tends to make bigger blocks of work.

I'm wondering if we need to put some of these on the road map and tying them to some goals.

Say 2.0 is a fat, transitional release that allows us to strip out the pieces so by 3.0 fog is modular with cleaner interfaces so providers can work independently.

So the 1.x series works as is, 2.x works as both and as you say would be harder to follow. 3.x removes the 1.x interfaces.

Somewhere in 2.x we pull out core and providers. After we've done that though it really will be tough to keep up with all the providers across several repositories!

@geemus
fog member

That seems pretty reasonable. I hadn't been too sure about how, version-wise, to handle the splitting out part. I'd hoped to start on some of that rather sooner than later though. At that point keeping things consistent becomes a bit less important (as each project can tend a bit more toward its own best practices and hopefully the very best practices will kind of rise to the top).

@tokengeek tokengeek added the summit label Mar 23, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment