Closed Bug 859516 Opened 11 years ago Closed 11 years ago

Add autosuggest JSON API as is available on the website

Categories

(Marketplace Graveyard :: Search, defect, P3)

Product:
Marketplace Graveyard
Component:
Search
Type:
defect

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
2013-05-09

People

(Reporter: mikedeboer, Assigned: ashort)

References

Details

(Whiteboard: p=2) 

While integrating AMO autocomplete add-on search into the Firefox Add-on Manager (see bug 712514), we noticed we have the space to add rating stars to the autocomplete results.

I don't know what the hit on the backend would be for this, I can only say it would be a welcome addition to our UI for users.
Blocks: 712514
URL I'm calling is the same as the autocomplete box on the addons.mozilla.org website uses:

https://addons.mozilla.org/en-US/firefox/search/suggestions?q=tr&appver=23.0a1&platform=Darwin&cat=all

If you need more info, please let me know!
Summary: Add 'rating' to autosuggest API JSON results → Add autosuggest JSON API as is available on the website
After discussing this on IRC in #amo with robhudson and cvan:

* A new API endpoint is the most desired option. It can be a direct copy of the API that lives on the URL as mentioned in comment 1.
* The return type would preferably be JSON as well.
* One exception: we (firefox-desktop team, moi included) would like the 'rating' field to be added to the results.

Example API endpoint:

https://services.addons.mozilla.org/en-US/firefox/api/1.5/search/suggestions/trololo/all/Darwin/23.0alpha/normal?src=firefox

Example output:

[{
  "url": "/en-US/firefox/addon/TrololoComment/?src=ss",
  "id": "384205",
  "name": "Trololo",
  "icon": "https://addons.cdn.mozilla.net/img/uploads/addon_icons/384/384205-32.png?modified=1360189623",
  "rating": 4.2565
}, {
  "url": "/en-US/firefox/addon/fb-troll-faces/?src=ss",
  "id": "389685",
  "name": "Troll Faces for Facebook Chat",
  "icon": "https://addons.cdn.mozilla.net/img/uploads/addon_icons/389/389685-32.png?modified=1346106022",
  "rating": 0
}, {
  "url": "/en-US/firefox/addon/trololololo-man/?src=ss",
  "id": "120008",
  "name": "Trololololo Man",
  "icon": "https://addons.cdn.mozilla.net/img/uploads/addon_icons/120/120008-32.png?modified=1281071023",
  "rating": 3.25
}]
Preferably we'd not return a JSON array, which has CSRF attack vectors. There's no sensitive data here but as good practice I'd prefer to see something like:

{"suggestions":
  [{
    "url": "/en-US/firefox/addon/TrololoComment/?src=ss",
    "id": "384205",
    "name": "Trololo",
    "icon": "https://addons.cdn.mozilla.net/img/uploads/addon_icons/384/384205-32.png?modified=1360189623",
    "rating": 4.2565
  },
  ...
  ]
}
Rob: bueno! I'd be fine with that, naturally.
Component: Integration → Search
Why do you prefer a separate API instead of adding rating to the existing one?
Wil: I don't prefer a separate API at all! I'd love to have rating added to the existing one. In bug 712514 I implemented the autocomplete using the API as used by the website (see comment 1).
cvan told me that your public, user-facing URLs are not primed for the load of Firefox, that's why I suggested a separate API to be able to make it run on the cluster.
That sounds like we should fix the existing API to scale. ;)

Cvan:  what made you say that?  Does it not use ES?  /me hasn't seen the code
(In reply to Wil Clouser [:clouserw] from comment #7)
> That sounds like we should fix the existing API to scale. ;)
> 
> Cvan:  what made you say that?  Does it not use ES?  /me hasn't seen the code

https://addons.mozilla.org/en-US/firefox/search/suggestions?q=adblock is not being served from SAMO. And I would be afraid to introduce that URL as a new pref in `about:config`: http://f.cl.ly/items/0O0U403G3x461r1G0c1Q/Screen%20Shot%202013-04-08%20at%206.37.00%20PM.png

https://services.addons.mozilla.org/en-US/firefox/api/1.5/search/adblock gives close to what Mike wants, but it's in XML (incidentally, returning ?format=json would be trivial to implement) and overly verbose.

Either way, this sounds like an extension to the existing search suggestions, and we should probably avoid returning rating metadata in the generic search suggestions that we use today on the user-facing AMO pages - should keep those responses as small as humanly possible.

New API endpoint - let's do this. This shouldn't take too long.
(In reply to Wil Clouser [:clouserw] from comment #7)
> Cvan:  what made you say that?  Does it not use ES?  /me hasn't seen the code

We also make assumptions that our normal view code isn't being used outside of AMO -- that's what the APIs are for. I'd be concerned that if this were used in Firefox it may break if/when AMO is refactored in the future. Where as the APIs we are more careful b/c they're there with the intention of use by outside consumers.

I agree with cvan, it shouldn't take long.
Ah, I see.  With an end goal of moving AMO to use the SAMO API and getting rid of the current calls, yeah, I agree with that.
Priority: -- → P3
Mike: In addition to rating, don't we also want more icon sizes?
Whiteboard: p=2
Assignee: nobody → ashort
Yes, if different icon sizes can not be immediately deduced from the returned icon url, that would be something to add in this API. Please?
(In reply to Mike de Boer [:mikedeboer] from comment #12)
> Yes, if different icon sizes can not be immediately deduced from the
> returned icon url,

We *need* to avoid inspecting URLs - the API needs to not be tied to AMO specifics, as we support alternate implementations/installations of the API for other apps.
(In reply to Blair McBride [:Unfocused] (Back from the dead. Mostly.) from comment #13)
> (In reply to Mike de Boer [:mikedeboer] from comment #12)
> > Yes, if different icon sizes can not be immediately deduced from the
> > returned icon url,
> 
> We *need* to avoid inspecting URLs - the API needs to not be tied to AMO
> specifics, as we support alternate implementations/installations of the API
> for other apps.

Please give a spec or example output of what you'd like to see.  Comment 3 is good, but it sounds like you're adding more now.
no, this is fine atm. Since the icons are 32x32 pixels already, we're good!
https://github.com/mozilla/zamboni/commit/0e8647bcde
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
This is tracebacking at https://marketplace-dev.allizom.org/search/suggestions

http://sentry.dmz.phx1.mozilla.com/addons/marketplace-dev/group/13681/

TypeError: __init__() got an unexpected keyword argument 'ratings' - amo.decorators in wrapper

Stacktrace (most recent call last):

  File "django/core/handlers/base.py", line 111, in get_response
    response = callback(request, *callback_args, **callback_kwargs)
  File "amo/decorators.py", line 127, in wrapper
    response = func(*args, **kw)
  File "search/views.py", line 277, in ajax_search_suggestions
    suggester = suggesterClass(request, ratings=False)

Types:	
TypeError
Value:	
__init__() got an unexpected keyword argument 'ratings'
Location:	search/views.py in ajax_search_suggestions , line 277
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
https://github.com/mozilla/zamboni/commit/1f315d95
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → FIXED
Sorry to reopen, but I'm nervous about this scaling.  Are you comfortable this can scale like crazy?  I'd like to see a waffle sample we can flip to adjust percentage of queries (return 503s for failures).  What do you think?

Logging number of requests in statsd would be interesting too, but not a requirement.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
One day when we upgrade elasticsearch to 0.90 or above we should use the suggest api:
http://www.elasticsearch.org/guide/reference/api/search/suggest/

For scaling we could potentially query elasticsearch directly and avoid the db lookup, assuming all the fields we need to return are in elasticsearch already. Otherwise we'd need to add them to avoid both a ES hit and a DB hit.

https://github.com/mozilla/zamboni/commit/38162329
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → FIXED
Reopening for comment 19
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
The original search suggestions also adds in categories and apps, which I would guess we don't want to do in the add-on manager:
https://github.com/mozilla/zamboni/blob/master/apps/search/views.py#L295-L330

That also is a cause for extra database queries we don't need, as well as 500 server errors when you search for things you know will find a category (e.g. "book") or an app (e.g. "fire") because those don't have ratings.

For scaling I'd try to make sure we're not hitting the database at all and only hit ES.
https://github.com/mozilla/zamboni/commit/daec432bbe8
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → FIXED
Reopening for comment 19 and comment 23
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Thanks!  This follows our normal cache headers (meaning, no front-end caching)?

Mike: If you get a 503 response it'd be awesome if you could ignore it gracefully and do some backoff - even simple backoff, like, don't try again for 15 minutes.
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2013-05-09
Wil: is this API now available and live?
Flags: needinfo?(clouserw)
Wooohooo!! Thanks! I will keep track of 503s in bug 712514.
(In reply to Wil Clouser [:clouserw] from comment #31)
> Looks like it 
> https://addons.mozilla.org/en-US/firefox/api/1.5/search_suggestions/?q=fire

Wil, that looks like a URL from the regular AMO site, not SAMO... is that intentional?
Flags: needinfo?(clouserw)
Looks like you can just use the SAMO domain instead with the same URL structure.  That's a good idea - thanks. :)
Flags: needinfo?(clouserw)
Blocks: 948367
You need to log in before you can comment on or make changes to this bug.