databricks / spark-pr-dashboard

Dashboard to aid in Spark pull request reviews
spark-prs.appspot.com
Apache License 2.0
54 stars 46 forks source link

Comment pagination broken due to ETags #32

Open JoshRosen opened 10 years ago

JoshRosen commented 10 years ago

For very long discussion threads, it looks like we miss new comments. I thought I had addressed this through the pagination helper function, but it looks like we're running into problems due to ETags. If nothing changes on the first page of comments, then I think its ETag is the same and we end up skipping subsequent fixes.

For now, I've updated the live site with a hotfix to ignore those ETags (since we're well under my GitHub API key's rate-limit at the moment). We should implement a real fix, though. One approach would be to request the comments in reverse-order or to check the ETag of the last comment page when deciding whether to fetch all of them.

JoshRosen commented 9 years ago

It turns out that it's not possible to request the comments in a (reverse) sorted order, so there's not a trivial fix for this.