Skip to content

Commit

Permalink
Forward etag and cache-control (and set proper mime) for /issues/lables.
Browse files Browse the repository at this point in the history 
Issue #239. and Issue #241.
  • Loading branch information
Mike Taylor committed Aug 28, 2014
1 parent 7667be3 commit f5ef429
Showing 1 changed file with 9 additions and 5 deletions.
14 changes: 9 additions & 5 deletions webcompat/api/endpoints.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
back to GitHub'''

import json
from flask import abort, Blueprint, g, request, session
from flask import abort, Blueprint, g, request, session, make_response
from flask.ext.github import GitHubError
from webcompat import github, app, cache
from ..issues import REPO_URI, proxy_request, filter_untriaged
Expand Down Expand Up @@ -174,8 +174,12 @@ def get_repo_labels():
# Chop off /issues. Someone feel free to refactor the ISSUES_REPO_URI.
labels_uri = app.config['ISSUES_REPO_URI'][:-7]
if g.user:
labels = github.get('repos/{0}/labels'.format(labels_uri))
labels = github.raw_request('GET', 'repos/{0}/labels'.format(labels_uri))
response = make_response(json.dumps(labels.json()))
response.headers['etag'] = labels.headers.get('etag')
response.headers['cache-control'] = labels.headers.get('cache-control')
response.headers['content-type'] = 'application/json'
return response
else:
labels = proxy_request('get', '/labels', uri=labels_uri,
token='labelbot')
return json.dumps(labels)
# only authed users should be hitting this endpoint
abort(401)

3 comments on commit f5ef429

@karlcow
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks almost good. ;)

  1. HTML page is loaded
  2. JS-client makes a request at Host: webcompat.com, GET /issues/labels FLask-server
  3. Flask-client makes a request at Host: api.github.com, GET repos/webcompat/labels and caches the answer
  4. Flask-server sends along to JS the response with the ETAG and cache-control headers.
  5. JS-client cache the ETAG.
  6. displays it in the HTML page
  7. Let's reload the page
  8. JS-client makes a request at Host: webcompat.com, GET /issues/labels with ETAG in the request If-None-Match:
  9. Flask-server should serve the cached answer if the function was called less than 600s ago. @cache.cached(timeout=600) (application cache != web cache), but ETAG does nothing here.
  10. if more than 600s ago, then Flask-client does the request to github anyway.
  11. Flask-server is sending back the fresh answer to JS-client

The issue: The ETAG aka If-None-Match: is not sent to GitHub in the raw_request, which means github doesn't know if it should send you a 304 Not Modified or a 200 OK

It's the tricky part of WebCompat being an intermediary proxy which is sometimes acting as a server and sometimes as a client.
I'm not sure it will work how you intend it to work.

@karlcow
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How to fix:

After the step 8:

  1. Flask-server grabs the value of If-None-Match from the JS-client request
  2. Flask-client creates an HTTP request to Github with the header If-None-Match and the value we captured
  3. Github-server replies with a 304 or a 200 to Flask-client
  4. Flask-server sends the response back to the JS-client, which will be one of these
    1. a 304 Not Modified
    2. a 200 Full response with a new ETAG.

Note that in this case the cache timeout might get into the way. If the labels have been changed in the last 600s since the last time the function has been called it will send back the application cached label view (!= web cache). Also, I wonder how the Flask cache is working with private information, I guess it doesn't matter because there is only one authenticated user which is webcompat account.

@miketaylr
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How to fix:

🍰

Please sign in to comment.