Chromium Code Reviews| Index: appengine/findit/common/rietveld.py |
| diff --git a/appengine/findit/common/rietveld.py b/appengine/findit/common/rietveld.py |
| index fab452cbd107029c3f3be7d7ac718975a4d3200a..d4b53abd12e756466724dd9c063ed555f21742c4 100644 |
| --- a/appengine/findit/common/rietveld.py |
| +++ b/appengine/findit/common/rietveld.py |
| @@ -2,18 +2,28 @@ |
| # Use of this source code is governed by a BSD-style license that can be |
| # found in the LICENSE file. |
| +# TODO(wrengr): this file is only used by waterfall. We should either |
|
stgao
2016/10/13 05:47:32
This should go to an infra/ directory in the new c
wrengr
2016/10/27 17:28:00
Is there a bug ticket for doing the new layout? If
|
| +# move it to that directory, or else factor out the dependency on |
| +# HttpClientAppengine by abstracting over the HTTP_CLIENT member. The |
|
stgao
2016/10/13 05:47:32
Regarding the HTTP_CLIENT member, we could pass it
wrengr
2016/10/27 17:28:00
Acknowledged.
|
| +# only thing we use/need HTTP_CLIENT for is to call the Post method, |
| +# which could just as well be rewritten to use the __call__ method |
| +# instead; hence, we could just pass an appropriate callable object to |
| +# the constructor (incidentally allowing multiple Rietveld objects to |
| +# share the same HTTP_CLIENT, if desired) rather than allocating our own |
| +# one automatically. Does rietveld itself actually require appengine? |
| +# Or is that just an incidental circumstance about our particular use |
| +# of it? |
| import logging |
| import re |
| import urlparse |
| -from common.codereview import CodeReview |
|
stgao
2016/10/13 05:47:32
Please check my previous comment on why we wanted
|
| from common.http_client_appengine import HttpClientAppengine |
| _RIETVELD_ISSUE_NUMBER_RE = re.compile('^/(\d+)/?.*') |
| -class Rietveld(CodeReview): |
| +class Rietveld(object): |
| """The implementation of CodeReview interface for Rietveld.""" |
| HTTP_CLIENT = HttpClientAppengine(follow_redirects=False) |