846347 - Configmanized middleware doesn't allow implementation overriding

Status: RESOLVED FIXED

(Socorro Graveyard :: Middleware, defect, P1)

Description

[[DEACTIVATED] Adrian Gaudebert]

The current version of the middleware allows to force the usage of a specific implementation at runtime, using the `force_api_impl` parameter. For example, calling /search/products/.../force_api_impl/elasticsearch/ will force the service to be executed using elasticsearch instead of whatever the default is, unless there is no implementation with elasticsearch for that service. 

This is not possible anymore with the new middleware app, and we shall fix it.

Comment 1

[Peter Bengtsson [:peterbe]]

I thought you said the use case for `force_api_impl` was entirely for QA purposes. 

Why do you need to switch implementation during runtime?

Comment 2

[[DEACTIVATED] Adrian Gaudebert]

That's for QA indeed. I don't remember, did we agree to remove it? I think we want to be able to switch to elasticsearch dynamically, especially to compare results between both implementations. I am mainly thinking about Search here, as we will start testing it very soon.

Comment 3

[Peter Bengtsson [:peterbe]]

Yes we did agree to ditch the runtime switch feature because we were going to get a much more configurable backend. 

The truth is, I don't think it's going to be really easy to add this back in because the list of classes is define when the application is instanciated, i.e. before the requests come in. In other words we couldn't use `?force_api_impl=something`.

What we could do is override the URL lookup. 
Perhaps here, https://github.com/mozilla/socorro/blob/master/socorro/middleware/middleware_app.py#L287
we do that first and then we do something like to make `/_impl/es/product/(.*)` a valid URL if there is class for `elasticsearch` with the class path `products.Products`. 

But it won't be particularly pretty. Is it not an option override the .ini file temporarily for QA and then reset it back? Something we could only really do on dev I suppose.

Comment 4

[[DEACTIVATED] Adrian Gaudebert]

I have started working on a solution for this, one that will work like the current one, I'll show you what I come up with in a pull request.

Comment 5

[[DEACTIVATED] Adrian Gaudebert]

Pull request: https://github.com/mozilla/socorro/pull/1106

Comment 6

[[DEACTIVATED] Adrian Gaudebert]

QA- because this would require UI work to be tested. If nothing breaks then this is good, we are adding a new functionality to the middleware that will be used during elasticsearch testing.

Comment 7

[[github robot]]

Commits pushed to master at https://github.com/mozilla/socorro

https://github.com/mozilla/socorro/commit/2a4f33b94cfc141b01610c77c308440379f7eff8
Fixes bug 846347 - Allowed implementation overriding in configman middleware app.

https://github.com/mozilla/socorro/commit/504ac2b2984a62e60e382f61893efbc97e3ad7f3
Merge pull request #1106 from AdrianGaudebert/846347-middleware-force-impl

Fixes bug 846347 - Allowed implementation overriding in configman middle...