Configmanized middleware doesn't allow implementation overriding

RESOLVED FIXED in 40

Status

Socorro Graveyard
Middleware
P1
normal
RESOLVED FIXED
5 years ago
2 years ago

People

(Reporter: adrian, Assigned: adrian)

Tracking

unspecified

Details

(Whiteboard: [qa-])

(Assignee)

Description

5 years ago
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.
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?
(Assignee)

Comment 2

5 years ago
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.
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.
(Assignee)

Comment 4

5 years ago
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.
(Assignee)

Comment 6

5 years ago
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.
Priority: -- → P1
Whiteboard: [qa-]
Target Milestone: --- → 39
(Assignee)

Updated

5 years ago
Target Milestone: 39 → 40

Comment 7

5 years ago
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...

Updated

5 years ago
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Assignee)

Updated

5 years ago
Blocks: 857055
Product: Socorro → Socorro Graveyard
You need to log in before you can comment on or make changes to this bug.