Add autosuggest JSON API as is available on the website

RESOLVED FIXED in 2013-05-09

Status

Marketplace
Search
P3
normal
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: mikedeboer, Assigned: ashort)

Tracking

2013-05-09
Points:
---
Dependency tree / graph

Details

(Whiteboard: p=2)

(Reporter)

Description

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

Updated

5 years ago
Blocks: 712514
(Reporter)

Comment 1

5 years ago
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!
(Reporter)

Updated

5 years ago
Summary: Add 'rating' to autosuggest API JSON results → Add autosuggest JSON API as is available on the website
(Reporter)

Comment 2

5 years ago
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
  },
  ...
  ]
}
(Reporter)

Comment 4

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

Comment 6

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

Updated

5 years ago
Assignee: nobody → ashort
(Reporter)

Comment 12

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

Comment 15

5 years ago
no, this is fine atm. Since the icons are 32x32 pixels already, we're good!
(Assignee)

Comment 16

5 years ago
https://github.com/mozilla/zamboni/commit/0e8647bcde
Status: NEW → RESOLVED
Last Resolved: 5 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 → ---
(Assignee)

Comment 18

5 years ago
https://github.com/mozilla/zamboni/commit/1f315d95
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 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.
(Assignee)

Comment 22

5 years ago

https://github.com/mozilla/zamboni/commit/38162329
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 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.
(Assignee)

Comment 26

5 years ago
https://github.com/mozilla/zamboni/commit/daec432bbe8
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 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
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2013-05-09
(Reporter)

Comment 30

5 years ago
Wil: is this API now available and live?
Flags: needinfo?(clouserw)
(Reporter)

Comment 32

5 years ago
Wooohooo!! Thanks! I will keep track of 503s in bug 712514.
(Reporter)

Comment 33

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

Updated

5 years ago
Blocks: 948367
You need to log in before you can comment on or make changes to this bug.