Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2114073002:
Add HttpsMonitor (Closed)
|
Created:
4 years, 5 months ago by tnn Modified:
4 years, 5 months ago Reviewers:
Sergey Berezin CC:
chromium-reviews, infra-reviews+infra_chromium.org Base URL:
https://chromium.googlesource.com/infra/infra.git@master Target Ref:
refs/heads/master Project:
infra Visibility:
Public. |
DescriptionAdd HttpsMonitor
BUG=625226
R=sergeyberezin@chromium.org
Committed: https://chromium.googlesource.com/infra/infra/+/2f08d02cb4cb2bff8b9c8885487792cc213675cf
Patch Set 1 : Add HttpsMonitor #
Total comments: 10
Patch Set 2 : Add retryable and other things #
Total comments: 25
Patch Set 3 : Fix #
Total comments: 8
Patch Set 4 : Fix nits #Patch Set 5 : Fix coverage #
Messages
Total messages: 35 (16 generated)
Patchset #1 (id:1) has been deleted
Patchset #1 (id:20001) has been deleted
Patchset #1 (id:40001) has been deleted
Patchset #1 (id:60001) has been deleted
Patchset #1 (id:80001) has been deleted
LGTM + a few small comments. Thanks! https://codereview.chromium.org/2114073002/diff/100001/infra_libs/ts_mon/comm... File infra_libs/ts_mon/common/monitors.py (right): https://codereview.chromium.org/2114073002/diff/100001/infra_libs/ts_mon/comm... infra_libs/ts_mon/common/monitors.py:102: resp, content = self._http.request(self._endpoint, method='POST', You also want to retry this automatically a few times (5 is plenty) for non-4xx error codes, and for pretty much all exceptions. Transient network failures are quite common. https://codereview.chromium.org/2114073002/diff/100001/infra_libs/ts_mon/comm... infra_libs/ts_mon/common/monitors.py:105: logging.warning("HttpsMonitor.send doesn't get the status 200: %s %s\n", nit: I'd rephrase this as "HttpsMonitor.send received status %d: %s %s\n" https://codereview.chromium.org/2114073002/diff/100001/infra_libs/ts_mon/comm... File infra_libs/ts_mon/common/pb_to_popo.py (right): https://codereview.chromium.org/2114073002/diff/100001/infra_libs/ts_mon/comm... infra_libs/ts_mon/common/pb_to_popo.py:12: obj[field.name] = list(_FD_TO_JSON[field.type](v) for v in value) nit: just to be on the safe side, I'd use _FD_TO_JSON.get(field.type, unicode). If a proto changes and gets a new type, at least the monitoring code won't crash the application.
https://codereview.chromium.org/2114073002/diff/100001/infra_libs/ts_mon/comm... File infra_libs/ts_mon/common/monitors.py (right): https://codereview.chromium.org/2114073002/diff/100001/infra_libs/ts_mon/comm... infra_libs/ts_mon/common/monitors.py:102: resp, content = self._http.request(self._endpoint, method='POST', On 2016/07/01 22:05:21, Sergey Berezin wrote: > You also want to retry this automatically a few times (5 is plenty) for non-4xx > error codes, and for pretty much all exceptions. Transient network failures are > quite common. For transient network failures, it retries 2 times (https://github.com/jcgregorio/httplib2/blob/master/python2/httplib2/__init__....) For something like 5XX, I don't think it retries. What's the cleanest way of adding retrying logic? My first thought is that retrying should be in a separate class like httplib2_utils.RetryableHttp. https://codereview.chromium.org/2114073002/diff/100001/infra_libs/ts_mon/comm... File infra_libs/ts_mon/common/pb_to_popo.py (right): https://codereview.chromium.org/2114073002/diff/100001/infra_libs/ts_mon/comm... infra_libs/ts_mon/common/pb_to_popo.py:12: obj[field.name] = list(_FD_TO_JSON[field.type](v) for v in value) On 2016/07/01 22:05:21, Sergey Berezin wrote: > nit: just to be on the safe side, I'd use _FD_TO_JSON.get(field.type, unicode). > If a proto changes and gets a new type, at least the monitoring code won't crash > the application. If a proto changes, crashing an application is better for uncovering a problem (epsecially when we do canarying first). Otherwise it would silently sending errorneous metrics. However, I don't have a strong feeling about this. Please let me know what you think.
On 2016/07/01 22:31:40, tanin wrote: > https://codereview.chromium.org/2114073002/diff/100001/infra_libs/ts_mon/comm... > File infra_libs/ts_mon/common/monitors.py (right): > > https://codereview.chromium.org/2114073002/diff/100001/infra_libs/ts_mon/comm... > infra_libs/ts_mon/common/monitors.py:102: resp, content = > self._http.request(self._endpoint, method='POST', > On 2016/07/01 22:05:21, Sergey Berezin wrote: > > You also want to retry this automatically a few times (5 is plenty) for > non-4xx > > error codes, and for pretty much all exceptions. Transient network failures > are > > quite common. > > For transient network failures, it retries 2 times > (https://github.com/jcgregorio/httplib2/blob/master/python2/httplib2/__init__....) > > For something like 5XX, I don't think it retries. What's the cleanest way of > adding retrying logic? > > My first thought is that retrying should be in a separate class like > httplib2_utils.RetryableHttp. > > https://codereview.chromium.org/2114073002/diff/100001/infra_libs/ts_mon/comm... > File infra_libs/ts_mon/common/pb_to_popo.py (right): > > https://codereview.chromium.org/2114073002/diff/100001/infra_libs/ts_mon/comm... > infra_libs/ts_mon/common/pb_to_popo.py:12: obj[field.name] = > list(_FD_TO_JSON[field.type](v) for v in value) > On 2016/07/01 22:05:21, Sergey Berezin wrote: > > nit: just to be on the safe side, I'd use _FD_TO_JSON.get(field.type, > unicode). > > If a proto changes and gets a new type, at least the monitoring code won't > crash > > the application. > > If a proto changes, crashing an application is better for uncovering a problem > (epsecially when we do canarying first). Otherwise it would silently sending > errorneous metrics. > > However, I don't have a strong feeling about this. Please let me know what you > think. A friendly ping after a long weekend.
https://codereview.chromium.org/2114073002/diff/100001/infra_libs/ts_mon/comm... File infra_libs/ts_mon/common/monitors.py (right): https://codereview.chromium.org/2114073002/diff/100001/infra_libs/ts_mon/comm... infra_libs/ts_mon/common/monitors.py:102: resp, content = self._http.request(self._endpoint, method='POST', On 2016/07/01 22:31:39, tanin wrote: > On 2016/07/01 22:05:21, Sergey Berezin wrote: > > You also want to retry this automatically a few times (5 is plenty) for > non-4xx > > error codes, and for pretty much all exceptions. Transient network failures > are > > quite common. > > For transient network failures, it retries 2 times > (https://github.com/jcgregorio/httplib2/blob/master/python2/httplib2/__init__....) > > For something like 5XX, I don't think it retries. What's the cleanest way of > adding retrying logic? > > My first thought is that retrying should be in a separate class like > httplib2_utils.RetryableHttp. For a shared implementation that would indeed be great. I don't think we use plain httplib2 calls anywhere else though - only as part of googleapiclient calls that have retries built in. So feel free to just wrap this in a retry loop right here if it's simpler. https://codereview.chromium.org/2114073002/diff/100001/infra_libs/ts_mon/comm... File infra_libs/ts_mon/common/pb_to_popo.py (right): https://codereview.chromium.org/2114073002/diff/100001/infra_libs/ts_mon/comm... infra_libs/ts_mon/common/pb_to_popo.py:12: obj[field.name] = list(_FD_TO_JSON[field.type](v) for v in value) On 2016/07/01 22:31:39, tanin wrote: > On 2016/07/01 22:05:21, Sergey Berezin wrote: > > nit: just to be on the safe side, I'd use _FD_TO_JSON.get(field.type, > unicode). > > If a proto changes and gets a new type, at least the monitoring code won't > crash > > the application. > > If a proto changes, crashing an application is better for uncovering a problem > (epsecially when we do canarying first). Otherwise it would silently sending > errorneous metrics. > > However, I don't have a strong feeling about this. Please let me know what you > think. For application code a crash may be preferred in some cases. But this is monitoring code, and a bug in it shouldn't crash all the applications in the fleet. I'd rather add excessive fault tolerance, log errors, and possibly corrupt monitoring data, but at least the app itself should continue to work normally and serve its users.
https://codereview.chromium.org/2114073002/diff/100001/infra_libs/ts_mon/comm... File infra_libs/ts_mon/common/monitors.py (right): https://codereview.chromium.org/2114073002/diff/100001/infra_libs/ts_mon/comm... infra_libs/ts_mon/common/monitors.py:102: resp, content = self._http.request(self._endpoint, method='POST', On 2016/07/06 17:18:39, Sergey Berezin wrote: > On 2016/07/01 22:31:39, tanin wrote: > > On 2016/07/01 22:05:21, Sergey Berezin wrote: > > > You also want to retry this automatically a few times (5 is plenty) for > > non-4xx > > > error codes, and for pretty much all exceptions. Transient network failures > > are > > > quite common. > > > > For transient network failures, it retries 2 times > > > (https://github.com/jcgregorio/httplib2/blob/master/python2/httplib2/__init__....) > > > > For something like 5XX, I don't think it retries. What's the cleanest way of > > adding retrying logic? > > > > My first thought is that retrying should be in a separate class like > > httplib2_utils.RetryableHttp. > > For a shared implementation that would indeed be great. I don't think we use > plain httplib2 calls anywhere else though - only as part of googleapiclient > calls that have retries built in. So feel free to just wrap this in a retry loop > right here if it's simpler. Done. https://codereview.chromium.org/2114073002/diff/100001/infra_libs/ts_mon/comm... infra_libs/ts_mon/common/monitors.py:105: logging.warning("HttpsMonitor.send doesn't get the status 200: %s %s\n", On 2016/07/01 22:05:21, Sergey Berezin wrote: > nit: I'd rephrase this as "HttpsMonitor.send received status %d: %s %s\n" Done. https://codereview.chromium.org/2114073002/diff/100001/infra_libs/ts_mon/comm... File infra_libs/ts_mon/common/pb_to_popo.py (right): https://codereview.chromium.org/2114073002/diff/100001/infra_libs/ts_mon/comm... infra_libs/ts_mon/common/pb_to_popo.py:12: obj[field.name] = list(_FD_TO_JSON[field.type](v) for v in value) On 2016/07/06 17:18:39, Sergey Berezin wrote: > On 2016/07/01 22:31:39, tanin wrote: > > On 2016/07/01 22:05:21, Sergey Berezin wrote: > > > nit: just to be on the safe side, I'd use _FD_TO_JSON.get(field.type, > > unicode). > > > If a proto changes and gets a new type, at least the monitoring code won't > > crash > > > the application. > > > > If a proto changes, crashing an application is better for uncovering a problem > > (epsecially when we do canarying first). Otherwise it would silently sending > > errorneous metrics. > > > > However, I don't have a strong feeling about this. Please let me know what you > > think. > > For application code a crash may be preferred in some cases. But this is > monitoring code, and a bug in it shouldn't crash all the applications in the > fleet. I'd rather add excessive fault tolerance, log errors, and possibly > corrupt monitoring data, but at least the app itself should continue to work > normally and serve its users. Fair enough. I'll use unicode and log something here. Thanks!
A bunch of more comments, mostly nits. Thanks for bearing with me :-) https://codereview.chromium.org/2114073002/diff/120001/infra_libs/__init__.py File infra_libs/__init__.py (right): https://codereview.chromium.org/2114073002/diff/120001/infra_libs/__init__.py... infra_libs/__init__.py:9: from infra_libs.httplib2_utils import RetryableHttp, InstrumentedHttp, HttpMock nit (spelling): Retriable https://codereview.chromium.org/2114073002/diff/120001/infra_libs/httplib2_ut... File infra_libs/httplib2_utils.py (right): https://codereview.chromium.org/2114073002/diff/120001/infra_libs/httplib2_ut... infra_libs/httplib2_utils.py:165: class RetryableHttp(object): nit (spelling): Retriable https://codereview.chromium.org/2114073002/diff/120001/infra_libs/httplib2_ut... infra_libs/httplib2_utils.py:165: class RetryableHttp(object): Inherit from httplib2.Http, and call base class's __init__(). https://codereview.chromium.org/2114073002/diff/120001/infra_libs/httplib2_ut... infra_libs/httplib2_utils.py:169: retrying_statuses=range(500,599)): For efficiency, I'd either make it a set(range(500, 599)), or a function. In fact, I'd prefer a function. Also, you might want to default to None, and assign the actual default as self.retrying_status = retrying_statuses or <default_value> It will keep pylint happier, and is friendlier to **kwargs pattern. https://codereview.chromium.org/2114073002/diff/120001/infra_libs/httplib2_ut... infra_libs/httplib2_utils.py:180: def request(self, uri, method="GET", body=None, *args, **kwargs): nit: use single quotes for strings if possible: 'GET'. (the InstrumentedHttp.request is using "wrong" quotes...) https://codereview.chromium.org/2114073002/diff/120001/infra_libs/httplib2_ut... infra_libs/httplib2_utils.py:184: uri, method, body, *args, **kwargs) nit: indent 4 spaces relative to the prev. line. https://codereview.chromium.org/2114073002/diff/120001/infra_libs/httplib2_ut... infra_libs/httplib2_utils.py:187: logging.info("RetryableHttp retries (%d) on receiving status: %d", nit (wording suggestion): logging.info( 'RetriableHttp: attempt %d received status %d, %s', i, response.status, 'final attempt' if i == self._max_tries else 'will retry') https://codereview.chromium.org/2114073002/diff/120001/infra_libs/httplib2_ut... infra_libs/httplib2_utils.py:191: socket.timeout, socket.error, socket.herror, socket.gaierror, nit: we always indent arguments to align: your_method( arg1, arg2, # note: 4-space indent, unlike 2-space for statements arg3, arg4) OR: your_method(arg1, arg2, arg3, arg4) https://codereview.chromium.org/2114073002/diff/120001/infra_libs/httplib2_ut... infra_libs/httplib2_utils.py:194: logging.warning("RetryableHttp fails on exception: %s", error) nit: 'single quotes' https://codereview.chromium.org/2114073002/diff/120001/infra_libs/httplib2_ut... infra_libs/httplib2_utils.py:197: logging.info("RetryableHttp retries (%d) on exception: %s", i, error) nit: 'single quotes', and similar wording suggestion as above. https://codereview.chromium.org/2114073002/diff/120001/infra_libs/test/httpli... File infra_libs/test/httplib2_utils_test.py (right): https://codereview.chromium.org/2114073002/diff/120001/infra_libs/test/httpli... infra_libs/test/httplib2_utils_test.py:137: self.http.http_obj.request = mock.Mock() Please try to use mock.create_autospec(..., spec_set=True), if possible. It will catch otherwise silent errors e.g. in my_mock.assert_haZ_calls(...) Alternatively, use self.assertEqual(my_mock.mock_calls, [<call list>]). This equally guards from the above pitfall, but wouldn't check the call signature. https://codereview.chromium.org/2114073002/diff/120001/infra_libs/ts_mon/comm... File infra_libs/ts_mon/common/monitors.py (right): https://codereview.chromium.org/2114073002/diff/120001/infra_libs/ts_mon/comm... infra_libs/ts_mon/common/monitors.py:91: httplib2_utils.InstrumentedHttp('acq-mon-api'))): nit: this is too complicated a default value. I'd suggest defaulting to None, and setting the default in the body as http = http or <default-value> https://codereview.chromium.org/2114073002/diff/120001/infra_libs/ts_mon/comm... infra_libs/ts_mon/common/monitors.py:106: logging.warning("HttpsMonitor.send received status %d: %s", resp, nit: use 'single quotes'.
https://codereview.chromium.org/2114073002/diff/120001/infra_libs/__init__.py File infra_libs/__init__.py (right): https://codereview.chromium.org/2114073002/diff/120001/infra_libs/__init__.py... infra_libs/__init__.py:9: from infra_libs.httplib2_utils import RetryableHttp, InstrumentedHttp, HttpMock On 2016/07/06 20:13:28, Sergey Berezin wrote: > nit (spelling): Retriable Me speak no English... Done. https://codereview.chromium.org/2114073002/diff/120001/infra_libs/httplib2_ut... File infra_libs/httplib2_utils.py (right): https://codereview.chromium.org/2114073002/diff/120001/infra_libs/httplib2_ut... infra_libs/httplib2_utils.py:165: class RetryableHttp(object): On 2016/07/06 20:13:29, Sergey Berezin wrote: > Inherit from httplib2.Http, and call base class's __init__(). Done. https://codereview.chromium.org/2114073002/diff/120001/infra_libs/httplib2_ut... infra_libs/httplib2_utils.py:169: retrying_statuses=range(500,599)): On 2016/07/06 20:13:28, Sergey Berezin wrote: > For efficiency, I'd either make it a set(range(500, 599)), or a function. In > fact, I'd prefer a function. > > Also, you might want to default to None, and assign the actual default as > > self.retrying_status = retrying_statuses or <default_value> > > It will keep pylint happier, and is friendlier to **kwargs pattern. Done. https://codereview.chromium.org/2114073002/diff/120001/infra_libs/httplib2_ut... infra_libs/httplib2_utils.py:180: def request(self, uri, method="GET", body=None, *args, **kwargs): On 2016/07/06 20:13:28, Sergey Berezin wrote: > nit: use single quotes for strings if possible: 'GET'. > > (the InstrumentedHttp.request is using "wrong" quotes...) Done. https://codereview.chromium.org/2114073002/diff/120001/infra_libs/httplib2_ut... infra_libs/httplib2_utils.py:184: uri, method, body, *args, **kwargs) On 2016/07/06 20:13:29, Sergey Berezin wrote: > nit: indent 4 spaces relative to the prev. line. I swear it looked like 4 spaces in my vim :S It turned out there was one tab character in here. Done. https://codereview.chromium.org/2114073002/diff/120001/infra_libs/httplib2_ut... infra_libs/httplib2_utils.py:187: logging.info("RetryableHttp retries (%d) on receiving status: %d", On 2016/07/06 20:13:28, Sergey Berezin wrote: > nit (wording suggestion): > logging.info( > 'RetriableHttp: attempt %d received status %d, %s', > i, response.status, > 'final attempt' if i == self._max_tries else 'will retry') > Done. https://codereview.chromium.org/2114073002/diff/120001/infra_libs/httplib2_ut... infra_libs/httplib2_utils.py:191: socket.timeout, socket.error, socket.herror, socket.gaierror, On 2016/07/06 20:13:28, Sergey Berezin wrote: > nit: we always indent arguments to align: > > your_method( > arg1, arg2, # note: 4-space indent, unlike 2-space for statements > arg3, arg4) > > OR: > > your_method(arg1, arg2, > arg3, arg4) Done. https://codereview.chromium.org/2114073002/diff/120001/infra_libs/httplib2_ut... infra_libs/httplib2_utils.py:194: logging.warning("RetryableHttp fails on exception: %s", error) On 2016/07/06 20:13:29, Sergey Berezin wrote: > nit: 'single quotes' Done. https://codereview.chromium.org/2114073002/diff/120001/infra_libs/httplib2_ut... infra_libs/httplib2_utils.py:197: logging.info("RetryableHttp retries (%d) on exception: %s", i, error) On 2016/07/06 20:13:28, Sergey Berezin wrote: > nit: 'single quotes', and similar wording suggestion as above. Done. https://codereview.chromium.org/2114073002/diff/120001/infra_libs/test/httpli... File infra_libs/test/httplib2_utils_test.py (right): https://codereview.chromium.org/2114073002/diff/120001/infra_libs/test/httpli... infra_libs/test/httplib2_utils_test.py:137: self.http.http_obj.request = mock.Mock() On 2016/07/06 20:13:29, Sergey Berezin wrote: > Please try to use mock.create_autospec(..., spec_set=True), if possible. It will > catch otherwise silent errors e.g. in > my_mock.assert_haZ_calls(...) > > Alternatively, use > > self.assertEqual(my_mock.mock_calls, [<call list>]). > > This equally guards from the above pitfall, but wouldn't check the call > signature. Done. https://codereview.chromium.org/2114073002/diff/120001/infra_libs/ts_mon/comm... File infra_libs/ts_mon/common/monitors.py (right): https://codereview.chromium.org/2114073002/diff/120001/infra_libs/ts_mon/comm... infra_libs/ts_mon/common/monitors.py:91: httplib2_utils.InstrumentedHttp('acq-mon-api'))): On 2016/07/06 20:13:29, Sergey Berezin wrote: > nit: this is too complicated a default value. I'd suggest defaulting to None, > and setting the default in the body as > > http = http or <default-value> Done. https://codereview.chromium.org/2114073002/diff/120001/infra_libs/ts_mon/comm... infra_libs/ts_mon/common/monitors.py:106: logging.warning("HttpsMonitor.send received status %d: %s", resp, On 2016/07/06 20:13:29, Sergey Berezin wrote: > nit: use 'single quotes'. Done.
LGTM, looks great! A few more final nits and we're done. https://codereview.chromium.org/2114073002/diff/140001/infra_libs/test/httpli... File infra_libs/test/httplib2_utils_test.py (right): https://codereview.chromium.org/2114073002/diff/140001/infra_libs/test/httpli... infra_libs/test/httplib2_utils_test.py:138: _MOCK_REQUEST = mock.call(mock.ANY, mock.ANY, mock.ANY, mock.ANY, mock.ANY, nit: mock.call(*([mock.ANY] * 9)) https://codereview.chromium.org/2114073002/diff/140001/infra_libs/test/httpli... infra_libs/test/httplib2_utils_test.py:153: ].__iter__() nit: it's cleaner to write ... = iter([...]) https://codereview.chromium.org/2114073002/diff/140001/infra_libs/ts_mon/comm... File infra_libs/ts_mon/common/monitors.py (right): https://codereview.chromium.org/2114073002/diff/140001/infra_libs/ts_mon/comm... infra_libs/ts_mon/common/monitors.py:94: httplib2_utils.InstrumentedHttp('acq-mon-api')) nit: 4 spaces from the start of the previous line. I.e.: http = f( arg1, ...) https://codereview.chromium.org/2114073002/diff/140001/infra_libs/ts_mon/comm... File infra_libs/ts_mon/common/test/pb_to_popo_test.py (right): https://codereview.chromium.org/2114073002/diff/140001/infra_libs/ts_mon/comm... infra_libs/ts_mon/common/test/pb_to_popo_test.py:20: name='m1', counter=200, task=task, nit: 4 spaces from the start of the previous line: metric1 = ...Data( name='m1', ...)
https://codereview.chromium.org/2114073002/diff/140001/infra_libs/test/httpli... File infra_libs/test/httplib2_utils_test.py (right): https://codereview.chromium.org/2114073002/diff/140001/infra_libs/test/httpli... infra_libs/test/httplib2_utils_test.py:138: _MOCK_REQUEST = mock.call(mock.ANY, mock.ANY, mock.ANY, mock.ANY, mock.ANY, On 2016/07/06 22:35:49, Sergey Berezin wrote: > nit: mock.call(*([mock.ANY] * 9)) Done. https://codereview.chromium.org/2114073002/diff/140001/infra_libs/test/httpli... infra_libs/test/httplib2_utils_test.py:153: ].__iter__() On 2016/07/06 22:35:49, Sergey Berezin wrote: > nit: it's cleaner to write ... = iter([...]) Done. https://codereview.chromium.org/2114073002/diff/140001/infra_libs/ts_mon/comm... File infra_libs/ts_mon/common/monitors.py (right): https://codereview.chromium.org/2114073002/diff/140001/infra_libs/ts_mon/comm... infra_libs/ts_mon/common/monitors.py:94: httplib2_utils.InstrumentedHttp('acq-mon-api')) On 2016/07/06 22:35:49, Sergey Berezin wrote: > nit: 4 spaces from the start of the previous line. I.e.: > > http = f( > arg1, ...) Done. https://codereview.chromium.org/2114073002/diff/140001/infra_libs/ts_mon/comm... File infra_libs/ts_mon/common/test/pb_to_popo_test.py (right): https://codereview.chromium.org/2114073002/diff/140001/infra_libs/ts_mon/comm... infra_libs/ts_mon/common/test/pb_to_popo_test.py:20: name='m1', counter=200, task=task, On 2016/07/06 22:35:49, Sergey Berezin wrote: > nit: 4 spaces from the start of the previous line: > > metric1 = ...Data( > name='m1', ...) Done.
Great, thanks! Let's land this.
On 2016/07/06 23:12:15, Sergey Berezin wrote: > Great, thanks! Let's land this. Thank you!!
The CQ bit was checked by tanin@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from sergeyberezin@chromium.org Link to the patchset: https://codereview.chromium.org/2114073002/#ps160001 (title: "Fix nits")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: Infra Linux Precise 32 Tester on master.tryserver.infra (JOB_FAILED, https://build.chromium.org/p/tryserver.infra/builders/Infra%20Linux%20Precise...)
Patchset #5 (id:180001) has been deleted
Patchset #5 (id:200001) has been deleted
Patchset #5 (id:220001) has been deleted
The CQ bit was checked by tanin@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by tanin@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from sergeyberezin@chromium.org Link to the patchset: https://codereview.chromium.org/2114073002/#ps240001 (title: "Fix coverage")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Add HttpsMonitor BUG=625226 R=sergeyberezin@chromium.org ========== to ========== Add HttpsMonitor BUG=625226 R=sergeyberezin@chromium.org Committed: https://chromium.googlesource.com/infra/infra/+/2f08d02cb4cb2bff8b9c888548779... ==========
Message was sent while issue was closed.
Committed patchset #5 (id:240001) as https://chromium.googlesource.com/infra/infra/+/2f08d02cb4cb2bff8b9c888548779... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
