New issue
Advanced search Search tips

Issue 917500 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Most Progress requests are 404s

Project Member Reported by qyears...@chromium.org, Dec 21

Issue description

If "no Tricium run" is the typical response, then perhaps it shouldn't really be considered an error case.

In most cases where the plugin makes a Progress request, we don't necessarily expect there to be a results, since we don't know whether Tricium is enabled for that repo.

Things to consider doing:
 - If there are any cases where the plugin is unnecessarily making requests, reduce the number of requests.
 - Perhaps we can first check whether the Tricium is enabled for the repo, and if not, return some other error.
 - Also, we could return empty results with a 200 response.

Personally, I think returning empty results with a 200 response would make sense, and doesn't reduce the amount of information. Empty results would mean "no results, or no results yet". The status code is not used by the plugin.
 
Components: Infra>Platform>Tricium
Project Member

Comment 2 by bugdroid1@chromium.org, Dec 22

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra/+/326cb1f4da7494fd872f56124444356deec5a1dc

commit 326cb1f4da7494fd872f56124444356deec5a1dc
Author: Quinten Yearsley <qyearsley@chromium.org>
Date: Sat Dec 22 02:20:10 2018

[tricium] In Progress RPC, use status OK for "no run".

The rationale here is that "no run" is the normal case, and it doesn't
mean that client did anything wrong, so it shouldn't be a 4xx error.
The client can easily see that there is no run because the response
is an empty response (with no run ID set) in this case.

This would also make the monitoring change for https://crbug/876124
simpler, and it would make it easier to find real client errors.

Also in this CL:
 - Change the logging in this case to be the URL of the change,
   which is more useful for debugging.
 - Remove a useless debug log line.

Bug:  917500 
Change-Id: I4e66b1cd6fabe0434410229b67ad5df3966c1419
Reviewed-on: https://chromium-review.googlesource.com/c/1389217
Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org>
Commit-Queue: Marc-Antoine Ruel <maruel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#19716}
[modify] https://crrev.com/326cb1f4da7494fd872f56124444356deec5a1dc/go/src/infra/tricium/appengine/frontend/rpc_progress_test.go
[modify] https://crrev.com/326cb1f4da7494fd872f56124444356deec5a1dc/go/src/infra/tricium/appengine/frontend/rpc_progress.go

Owner: qyears...@chromium.org
Status: Started (was: Available)
Status: Fixed (was: Started)

Sign in to add a comment