Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #590 - Fix conditional proxy requests #608

Merged
merged 3 commits into from
Apr 13, 2015
Merged

Fixes #590 - Fix conditional proxy requests #608

merged 3 commits into from
Apr 13, 2015

Conversation

miketaylr
Copy link
Member

Mike Taylor added 2 commits April 10, 2015 15:25
Which will be merged with the AUTH_HEADERS and sent to github.
This allows us to send conditional GET requests based on eTag
in the case of un-authed requests.
@miketaylr miketaylr changed the title Fixes #590 - Fix conditional requests for proxy requests Fixes #590 - Fix conditional proxy requests Apr 10, 2015
@miketaylr
Copy link
Member Author

Oops, Travis didn't like that at all.

@karlcow
Copy link
Member

karlcow commented Apr 13, 2015

ok let see. Starting the code.

→ python run.py
 * Running on http://localhost:5000/
 * Restarting with reloader

Without being logged.

[…]
127.0.0.1 - - [13/Apr/2015 09:25:02] "GET /issues/52 HTTP/1.1" 200 -
127.0.0.1 - - [13/Apr/2015 09:25:03] "GET /js/lib/models/comment.js HTTP/1.1" 200 -
127.0.0.1 - - [13/Apr/2015 09:25:03] "GET /js/lib/comments.js HTTP/1.1" 200 -
127.0.0.1 - - [13/Apr/2015 09:25:03] "GET /js/lib/labels.js HTTP/1.1" 200 -
127.0.0.1 - - [13/Apr/2015 09:25:03] "GET /js/lib/issues.js HTTP/1.1" 200 -
    [proxy_issue]  {'Accept': 'application/json', 'User-Agent': u'Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:39.0) Gecko/20100101 Firefox/39.0'}
    [proxy_request] 1  {'Accept': 'application/json', 'User-Agent': u'Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:39.0) Gecko/20100101 Firefox/39.0'}
    [proxy_request] 2  {'Accept': 'application/json', 'User-Agent': u'Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:39.0) Gecko/20100101 Firefox/39.0'}
127.0.0.1 - - [13/Apr/2015 09:25:15] "GET /api/issues/52 HTTP/1.1" 200 -
    [proxy_request] 1  {'Accept': 'application/json', 'User-Agent': u'Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:39.0) Gecko/20100101 Firefox/39.0'}
    [proxy_request] 2  {'Accept': 'application/json', 'User-Agent': u'Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:39.0) Gecko/20100101 Firefox/39.0'}
127.0.0.1 - - [13/Apr/2015 09:25:19] "GET /api/issues/52/comments HTTP/1.1" 200 -
127.0.0.1 - - [13/Apr/2015 09:26:10] "GET /issues?page=1&per_page=50&state=open&stage=all&sort=created&direction=desc HTTP/1.1" 200 -
127.0.0.1 - - [13/Apr/2015 09:26:10] "GET /js/lib/issue-list.js HTTP/1.1" 200 -
    [proxy_request] 1  None
    [proxy_request] 2  None
127.0.0.1 - - [13/Apr/2015 09:26:15] "GET /api/issues?page=1&per_page=50&state=open&stage=all&sort=created&direction=desc HTTP/1.1" 200 -
127.0.0.1 - - [13/Apr/2015 09:26:15] "GET /img/loader.gif HTTP/1.1" 200 -
127.0.0.1 - - [13/Apr/2015 09:26:22] "GET /issues/52 HTTP/1.1" 200 -
    [proxy_issue]  {'If-None-Match': 'W/"e26172aa128c873d559ab8ab9e06be4a"', 'Accept': 'application/json', 'User-Agent': u'Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:39.0) Gecko/20100101 Firefox/39.0'}
    [proxy_request] 1  {'If-None-Match': 'W/"e26172aa128c873d559ab8ab9e06be4a"', 'Accept': 'application/json', 'User-Agent': u'Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:39.0) Gecko/20100101 Firefox/39.0'}
    [proxy_request] 2  {'If-None-Match': 'W/"e26172aa128c873d559ab8ab9e06be4a"', 'Accept': 'application/json', 'User-Agent': u'Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:39.0) Gecko/20100101 Firefox/39.0'}
127.0.0.1 - - [13/Apr/2015 09:26:35] "GET /api/issues/52 HTTP/1.1" 304 -
    [proxy_request] 1  {'If-None-Match': 'W/"26e424a23e73cc21f7ae51364b59b0a5"', 'Accept': 'application/json', 'User-Agent': u'Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:39.0) Gecko/20100101 Firefox/39.0'}
    [proxy_request] 2  {'If-None-Match': 'W/"26e424a23e73cc21f7ae51364b59b0a5"', 'Accept': 'application/json', 'User-Agent': u'Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:39.0) Gecko/20100101 Firefox/39.0'}
127.0.0.1 - - [13/Apr/2015 09:26:41] "GET /api/issues/52/comments HTTP/1.1" 304 -

This is working well for JSON, aka here http://localhost:5000/api/issues/52, but not for the html, which is currently not being cached aka http://localhost:5000/issues/52.

request-52

Digging a bit further, and trying to print the before_request headers:

the HTML. not ok

[before_request]  Referer: http://localhost:5000/issues?page=1&per_page=50&state=open&stage=all&sort=created&direction=desc
Content-Length: 
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:39.0) Gecko/20100101 Firefox/39.0
Connection: keep-alive
Dnt: 1
Host: localhost:5000
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
Accept-Language: en,fr;q=0.7,en-US;q=0.3
Content-Type: 
Accept-Encoding: gzip, deflate


127.0.0.1 - - [13/Apr/2015 09:42:04] "GET /issues/52 HTTP/1.1" 200 -

The JSON. OK

[before_request]  Referer: http://localhost:5000/issues/52
Content-Length: 
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:39.0) Gecko/20100101 Firefox/39.0
Connection: keep-alive
If-None-Match: W/"e26172aa128c873d559ab8ab9e06be4a"
X-Requested-With: XMLHttpRequest
Dnt: 1
Host: localhost:5000
Accept: application/json
Accept-Language: en,fr;q=0.7,en-US;q=0.3
Content-Type: 
Accept-Encoding: gzip, deflate


[proxy_issue]  {'If-None-Match': 'W/"e26172aa128c873d559ab8ab9e06be4a"', 'Accept': 'application/json', 'User-Agent': u'Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:39.0) Gecko/20100101 Firefox/39.0'}
[proxy_request] 1  {'If-None-Match': 'W/"e26172aa128c873d559ab8ab9e06be4a"', 'Accept': 'application/json', 'User-Agent': u'Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:39.0) Gecko/20100101 Firefox/39.0'}
[proxy_request] 2  {'If-None-Match': 'W/"e26172aa128c873d559ab8ab9e06be4a"', 'Accept': 'application/json', 'User-Agent': u'Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:39.0) Gecko/20100101 Firefox/39.0'}
127.0.0.1 - - [13/Apr/2015 09:42:08] "GET /api/issues/52 HTTP/1.1" 304 -

For the purpose of #590 this is indeed solving the proxy_request.
But we are still missing a couple of things.

And the server is indeed not sending any information for caching such as ETAG. I guess the issue is here for http://localhost:5000/issues/52or do you manage everything on nginx side? Date is also kind of wrong (but that's a separate issue.)

Content-Length: 8853
Content-Type: text/html; charset=utf-8
Date: Mon, 13 Apr 2015 00:26:22 GMT
Server: Werkzeug/0.9.4 Python/2.7.6
  • +1 for proxy_request on /api/issues/ You solved it with elegance.

We need to fix the HTML. On the server itself, we have the same story.

request-52-server

Checking the HTTP headers, the server is sending:

Connection: keep-alive
Content-Encoding: gzip
Content-Type: text/html; charset=utf-8
Date: Mon, 13 Apr 2015 01:00:27 GMT
Server: nginx/1.1.19
Strict-Transport-Security: max-age=31536000
Transfer-Encoding: chunked
Vary: Accept-Encoding

Thinking…

What I propose Let's close this one and merge because it actually solves the JSON part. And let's see if we can improve the HTML side. OK?

karlcow added a commit that referenced this pull request Apr 13, 2015
Fixes #590 - Fix conditional proxy requests for JSON
@karlcow karlcow merged commit 6cef999 into master Apr 13, 2015
@miketaylr
Copy link
Member Author

Sounds good @karlcow -- thanks for checking in-depth. :)

@miketaylr miketaylr deleted the issues/590/1 branch April 13, 2015 14:49
@miketaylr miketaylr restored the issues/590/1 branch July 20, 2015 19:04
@miketaylr miketaylr deleted the issues/590/1 branch July 21, 2015 04:07
@karlcow karlcow added this to the HTTP Caching milestone Feb 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants