Implement interface to Rietveld.

Implement interface to Rietveld.

This is a simple python interface based on Rietveld's HTTP interface, which is
being accessed via OAuth2 tokens generated for a service account authenticated
via an RSA key.

R=agable@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=215841

[Mattias Nissler (ping if slow), 2013-08-02 09:58:10]

This is the python wrapper for OAuth-authenticated HTTP requests to rietveld.
Please review!

[agable, 2013-08-02 16:35:35]

https://codereview.chromium.org/20495003/diff/1/reviewbot/model/app_config
File reviewbot/model/app_config (right):

https://codereview.chromium.org/20495003/diff/1/reviewbot/model/app_config#ne...
reviewbot/model/app_config:1: from google.appengine.ext import ndb
This file appears to be a mistake.

https://codereview.chromium.org/20495003/diff/1/reviewbot/model/app_config.py
File reviewbot/model/app_config.py (right):

https://codereview.chromium.org/20495003/diff/1/reviewbot/model/app_config.py...
reviewbot/model/app_config.py:19: config = AppConfig(id = 'config')
nit: no spaces around = in function calls and definitions.

https://codereview.chromium.org/20495003/diff/1/reviewbot/rietveld.py
File reviewbot/rietveld.py (right):

https://codereview.chromium.org/20495003/diff/1/reviewbot/rietveld.py#newcode16
reviewbot/rietveld.py:16: """Raised on request errors."""
include explicit 'pass' here.

https://codereview.chromium.org/20495003/diff/1/reviewbot/rietveld.py#newcode68
reviewbot/rietveld.py:68: def add_inline_comment(self, issue_id, patchset_id,
patch_id, line, a_or_b,
How necessary is a_or_b? Though we haven't seen the code for it yet, it seems
like the automated bot is only ever going to post comments on the latest
patchset. When would it place comments on an older version?

https://codereview.chromium.org/20495003/diff/1/reviewbot/util.py
File reviewbot/util.py (right):

https://codereview.chromium.org/20495003/diff/1/reviewbot/util.py#newcode23
reviewbot/util.py:23: self.lookup = lookup
nit: self.lookup = lookup_fn

[Mattias Nissler (ping if slow), 2013-08-05 09:08:50]

PTAL

https://chromiumcodereview.appspot.com/20495003/diff/1/reviewbot/model/app_co...
File reviewbot/model/app_config (right):

https://chromiumcodereview.appspot.com/20495003/diff/1/reviewbot/model/app_co...
reviewbot/model/app_config:1: from google.appengine.ext import ndb
On 2013/08/02 16:35:35, Aaron Gable wrote:
> This file appears to be a mistake.

Yes, removed.

https://chromiumcodereview.appspot.com/20495003/diff/1/reviewbot/model/app_co...
File reviewbot/model/app_config.py (right):

https://chromiumcodereview.appspot.com/20495003/diff/1/reviewbot/model/app_co...
reviewbot/model/app_config.py:19: config = AppConfig(id = 'config')
On 2013/08/02 16:35:35, Aaron Gable wrote:
> nit: no spaces around = in function calls and definitions.

Done.

https://chromiumcodereview.appspot.com/20495003/diff/1/reviewbot/rietveld.py
File reviewbot/rietveld.py (right):

https://chromiumcodereview.appspot.com/20495003/diff/1/reviewbot/rietveld.py#...
reviewbot/rietveld.py:16: """Raised on request errors."""
On 2013/08/02 16:35:35, Aaron Gable wrote:
> include explicit 'pass' here.

Done.

https://chromiumcodereview.appspot.com/20495003/diff/1/reviewbot/rietveld.py#...
reviewbot/rietveld.py:68: def add_inline_comment(self, issue_id, patchset_id,
patch_id, line, a_or_b,
On 2013/08/02 16:35:35, Aaron Gable wrote:
> How necessary is a_or_b? Though we haven't seen the code for it yet, it seems
> like the automated bot is only ever going to post comments on the latest
> patchset. When would it place comments on an older version?

a_or_b determines whether the comment appears on the repository version or the
new version of the file. It's useful when something gets deleted in which case
there's no good location in the new file to attach the comment to.

https://chromiumcodereview.appspot.com/20495003/diff/1/reviewbot/util.py
File reviewbot/util.py (right):

https://chromiumcodereview.appspot.com/20495003/diff/1/reviewbot/util.py#newc...
reviewbot/util.py:23: self.lookup = lookup
On 2013/08/02 16:35:35, Aaron Gable wrote:
> nit: self.lookup = lookup_fn

Done.

[agable, 2013-08-05 17:38:00]

LGTM % nit and docstrings.

https://chromiumcodereview.appspot.com/20495003/diff/1/reviewbot/model/app_co...
File reviewbot/model/app_config.py (right):

https://chromiumcodereview.appspot.com/20495003/diff/1/reviewbot/model/app_co...
reviewbot/model/app_config.py:19: config = AppConfig(id = 'config')
On 2013/08/05 09:08:51, Mattias Nissler wrote:
> On 2013/08/02 16:35:35, Aaron Gable wrote:
> > nit: no spaces around = in function calls and definitions.
> 
> Done.

There are some other instances of this in rietveld.py

https://chromiumcodereview.appspot.com/20495003/diff/7001/reviewbot/rietveld.py
File reviewbot/rietveld.py (right):

https://chromiumcodereview.appspot.com/20495003/diff/7001/reviewbot/rietveld....
reviewbot/rietveld.py:20: class Rietveld(object):
Either have a class docstring here or a module docstring at the top of the file.

https://chromiumcodereview.appspot.com/20495003/diff/7001/reviewbot/rietveld....
reviewbot/rietveld.py:61: 'message_only': 0 if submit_inline_comments else 1,
This is the equivalent of differentiating between hitting "Reply" on the latest
message in the thread and hitting "Publish and Mail Comments", yes?

https://chromiumcodereview.appspot.com/20495003/diff/7001/reviewbot/util.py
File reviewbot/util.py (right):

https://chromiumcodereview.appspot.com/20495003/diff/7001/reviewbot/util.py#n...
reviewbot/util.py:20: class LazyDict(object):
They're pretty self explanatory but I like having docstrings for all classes.

[Mattias Nissler (ping if slow), 2013-08-05 18:40:47]

Thanks for the review. New version is up, I'll commit this tomorrow.

https://chromiumcodereview.appspot.com/20495003/diff/1/reviewbot/model/app_co...
File reviewbot/model/app_config.py (right):

https://chromiumcodereview.appspot.com/20495003/diff/1/reviewbot/model/app_co...
reviewbot/model/app_config.py:19: config = AppConfig(id = 'config')
On 2013/08/05 17:38:00, Aaron Gable wrote:
> On 2013/08/05 09:08:51, Mattias Nissler wrote:
> > On 2013/08/02 16:35:35, Aaron Gable wrote:
> > > nit: no spaces around = in function calls and definitions.
> > 
> > Done.
> 
> There are some other instances of this in rietveld.py

Fixed. (I'm surprised pylint doesn't catch those!)

https://chromiumcodereview.appspot.com/20495003/diff/7001/reviewbot/rietveld.py
File reviewbot/rietveld.py (right):

https://chromiumcodereview.appspot.com/20495003/diff/7001/reviewbot/rietveld....
reviewbot/rietveld.py:20: class Rietveld(object):
On 2013/08/05 17:38:00, Aaron Gable wrote:
> Either have a class docstring here or a module docstring at the top of the
file.

Done.

https://chromiumcodereview.appspot.com/20495003/diff/7001/reviewbot/rietveld....
reviewbot/rietveld.py:61: 'message_only': 0 if submit_inline_comments else 1,
On 2013/08/05 17:38:00, Aaron Gable wrote:
> This is the equivalent of differentiating between hitting "Reply" on the
latest
> message in the thread and hitting "Publish and Mail Comments", yes?

Correct.

https://chromiumcodereview.appspot.com/20495003/diff/7001/reviewbot/util.py
File reviewbot/util.py (right):

https://chromiumcodereview.appspot.com/20495003/diff/7001/reviewbot/util.py#n...
reviewbot/util.py:20: class LazyDict(object):
On 2013/08/05 17:38:00, Aaron Gable wrote:
> They're pretty self explanatory but I like having docstrings for all classes.

Done.

[Mattias Nissler (ping if slow), 2013-08-06 07:45:15]

Committed patchset #3 manually as r215841 (presubmit successful).