669699 - Allow the UI to choose what search implementation to use in the mware

Status: RESOLVED FIXED

(Socorro :: Webapp, task, P2)

Description

[[DEACTIVATED] Adrian Gaudebert]

For testing purpose mainly, it would be useful to allow to choose the implementation used when performing a search in the middleware. It would basically consist in a new parameter in the URL that says to force ElasticSearch or PostgreSQL.

Comment 1

[[DEACTIVATED] Adrian Gaudebert]

Attachment: Add an option for choosing the implementation + PEP8

This patch does a small refactor of the search service class, mainly to add the possibility to force the API implementation to use, secondarily to make it PEP8 compliant.

Comment 2

[[DEACTIVATED] Adrian Gaudebert]

Comment on attachment 551200 [details] [diff] [review]
Add an option for choosing the implementation + PEP8

Review of attachment 551200 [details] [diff] [review]:
-----------------------------------------------------------------

rhelmer is out and it's been waiting for review for weeks. lonnen?

Comment 3

[Lonnen :lonnen]

Comment on attachment 551200 [details] [diff] [review]
Add an option for choosing the implementation + PEP8

Post patch, line 11 should be a blank line. Fix that and this is good.

Comment 4

[[DEACTIVATED] Adrian Gaudebert]

Attachment: Add an option for choosing the implementation + PEP8 v2

I added the missing blank line and removed some uneeded ones. Chris can you review it once again please, I think nothing changed but the patch is very different from the previous one, so I would like to be sure before pushing.

Comment 5

[Lonnen :lonnen]

Comment on attachment 554273 [details] [diff] [review]
Add an option for choosing the implementation + PEP8 v2

The diffs are different but their output differences are exactly what you were expecting. Thanks

Comment 6

[[DEACTIVATED] Adrian Gaudebert]

Landed on trunk with commit r3472: http://code.google.com/p/socorro/source/detail?r=3472

Comment 7

[Matt Brandt [:mbrandt]]

QA verified. Using the forced_api_impl flag "es" and "pg" return the same results:

Each query returns 28 results. Digging down into the results and continuing to use the flag, the results appear the same.

https://crash-stats.allizom.org/query/query?product=Firefox&version=ALL%3AALL&range_value=1&range_unit=weeks&date=08%2F29%2F2011+10%3A27%3A00&query_search=signature&query_type=contains&query=UserCallWinProcCheckWow&reason=&build_id=&process_type=any&hang_type=any&forced_api_impl=es&do_query=1

https://crash-stats.allizom.org/query/query?product=Firefox&version=ALL%3AALL&range_value=1&range_unit=weeks&date=08%2F29%2F2011+10%3A27%3A00&query_search=signature&query_type=contains&query=UserCallWinProcCheckWow&reason=&build_id=&process_type=any&hang_type=any&forced_api_impl=matt&do_query=1

https://crash-stats.allizom.org/query/query?product=Firefox&version=ALL%3AALL&range_value=1&range_unit=weeks&date=08%2F29%2F2011+10%3A27%3A00&query_search=signature&query_type=contains&query=UserCallWinProcCheckWow&reason=&build_id=&process_type=any&hang_type=any&forced_api_impl=pg&do_query=1

https://crash-stats.allizom.org/query/query?product=Firefox&version=ALL%3AALL&range_value=1&range_unit=weeks&date=08%2F29%2F2011+10%3A27%3A00&query_search=signature&query_type=contains&query=UserCallWinProcCheckWow&reason=&build_id=&process_type=any&hang_type=any&forced_api_impl=es&do_query=1

Comment 8

[[DEACTIVATED] Adrian Gaudebert]

Guess what? Results didn't change because nothing changed in the back end, this forcing option was done for the middleware but is not accessible in the UI... I reopened and will fix this asap.

Comment 9

[Matt Brandt [:mbrandt]]

adrian, so I don't make the mistake again. Are there visual indicators in the app that QA can see so I don't mistakenly mark this as verified again?

Comment 10

[[DEACTIVATED] Adrian Gaudebert]

Attachment: Add an option for choosing the implementation in the UI

Add the option of choosing the implementation from the webapp, and fix the name of the parameter used in the middleware. 

To use it, add &force_api_impl=es or &force_api_impl=pg at the end of the url, respectively for ElasticSearch and PostgreSQL.

Comment 11

[[DEACTIVATED] Adrian Gaudebert]

Comment on attachment 557036 [details] [diff] [review]
Add an option for choosing the implementation in the UI

Review of attachment 557036 [details] [diff] [review]:
-----------------------------------------------------------------

Given to Chris Lonnen.

Comment 12

[Lonnen :lonnen]

Comment on attachment 557036 [details] [diff] [review]
Add an option for choosing the implementation in the UI

Going forward on git, it would be easier to point me to two separate commits: one for the code cleanup, a second for the changes. A patch is fine for this though.

Comment 13

[Lonnen :lonnen]

r+
merged:
https://github.com/mozilla/socorro/commit/38dc7da2ebfe4e8d6b615ad4b85dc684f0c8b2ff

Comment 14

[[DEACTIVATED] Adrian Gaudebert]

@Matt: To test this, do the exact same you did previously, make sure we have the same result (with some eventual very small differences), and also make sure that with ES you never see more than 1,000 results. 

Of course, you can just ask me anything you need.

Comment 15

[Matt Brandt [:mbrandt]]

After talking to adrian, we're reopening this to track the discussion.

Testing for the return of less than or equal to 1,000 results returns a 500.

https://crash-stats-dev.allizom.org/query/query?product=Firefox&version=ALL%3AALL&range_value=3&range_unit=weeks&date=08%2F29%2F2011+10%3A27%3A00&query_search=signature&query_type=contains&query=&reason=&build_id=&process_type=any&hang_type=any&forced_api_impl=es&do_query=1

Returns:
[11:13:59.541] GET https://crash-stats-dev.allizom.org/query/query?product=Firefox&version=ALL%3AALL&range_value=3&range_unit=weeks&date=08%2F29%2F2011+10%3A27%3A00&query_search=signature&query_type=contains&query=&reason=&build_id=&process_type=any&hang_type=any&forced_api_impl=es&do_query=1 [HTTP/1.1 500 Internal Server Error 31726ms]

And
https://crash-stats.allizom.org/query/query?product=Firefox&version=ALL%3AALL&range_value=3&range_unit=weeks&date=08%2F29%2F2011+10%3A27%3A00&query_search=signature&query_type=contains&query=&reason=&build_id=&process_type=any&hang_type=any&forced_api_impl=es&do_query=1

Results:

[11:33:50.481] GET https://crash-stats.allizom.org/query/query?product=Firefox&version=ALL%3AALL&range_value=3&range_unit=weeks&date=08%2F29%2F2011+10%3A27%3A00&query_search=signature&query_type=contains&query=&reason=&build_id=&process_type=any&hang_type=any&forced_api_impl=es&do_query=1 [HTTP/1.1 500 Internal Server Error 32029ms]

Comment 16

[[DEACTIVATED] Adrian Gaudebert]

Same as bug 682140, it is tested and working on my khan instance, we now are waiting for a link to ES to be opened from dev/stage.

Comment 17

[[DEACTIVATED] Adrian Gaudebert]

This was fixed by bug 857055.