Skip to content

Commit

Permalink
internal/frontend: use statusNotFoundInVersionMap for frontend fetch
Browse files Browse the repository at this point in the history
Previously, we used http.StatusProcessing and http.StatusNoContent to
indicate when a row did not exist in the version_map table.

Neither of these status codes fit well, so our own error code is defined
for the case when a module path has not been processed yet:
statusNotFoundInVersionMap.

For golang/go#36811
For golang/go#37002

Change-Id: Ia55972bfbfb1950ad82254d8f1082dda052bf376
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/245020
Run-TryBot: Julie Qiu <julie@golang.org>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
  • Loading branch information
julieqiu committed Jul 28, 2020
1 parent 8239dd3 commit f37c9e1
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 10 deletions.
8 changes: 4 additions & 4 deletions internal/frontend/details.go
Original file line number Diff line number Diff line change
Expand Up @@ -443,10 +443,10 @@ func (s *Server) servePathNotFoundPage(w http.ResponseWriter, r *http.Request, d
}
results := s.checkPossibleModulePaths(ctx, db, fullPath, requestedVersion, modulePaths, false)
for _, fr := range results {
if fr.status == http.StatusNoContent {
// If the result is StatusNoContent, it means that we
// haven't attempted to fetch this path before. Return an error
// page giving the user the option to fetch the path.
if fr.status == statusNotFoundInVersionMap {
// If the result is statusNotFoundInVersionMap, it means that
// we haven't attempted to fetch this path before. Return an
// error page giving the user the option to fetch the path.
return pathNotFoundErrorNew(fullPath, requestedVersion)
}
}
Expand Down
15 changes: 9 additions & 6 deletions internal/frontend/fetch.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,9 @@ var (
Description: "Frontend fetch request count",
TagKeys: []tag.Key{keyFetchStatus},
}
// statusNotFoundInVersionMap indicates that a row does not exist in
// version_map for the module_path and requested_version.
statusNotFoundInVersionMap = 470
)

// serveFetch checks if a requested path and version exists in the database.
Expand Down Expand Up @@ -185,7 +188,7 @@ func (s *Server) checkPossibleModulePaths(ctx context.Context, db *postgres.DB,
// have already attempted to fetch it in the past. If so, just
// return the result from that fetch process.
fr := checkForPath(ctx, db, fullPath, modulePath, requestedVersion)
if !shouldQueue || fr.status != http.StatusNoContent {
if !shouldQueue || fr.status != statusNotFoundInVersionMap {
results[i] = fr
return
}
Expand Down Expand Up @@ -283,7 +286,7 @@ func pollForPath(ctx context.Context, db *postgres.DB, pollEvery time.Duration,
ctx2, cancel := context.WithTimeout(ctx, pollEvery)
defer cancel()
fr = checkForPath(ctx2, db, fullPath, modulePath, requestedVersion)
if fr.status != http.StatusNoContent {
if fr.status != statusNotFoundInVersionMap {
return fr
}
}
Expand Down Expand Up @@ -318,7 +321,7 @@ func checkForPath(ctx context.Context, db *postgres.DB, fullPath, modulePath, re
// If an error is returned, there are two possibilities:
// (1) A row for this modulePath and version does not exist.
// This means that the fetch request is not done yet, so return
// http.StatusNoContent so the fetchHandler will call checkForPath
// statusNotFoundInVersionMap so the fetchHandler will call checkForPath
// again in a few seconds.
// (2) Something went wrong, so return that error.
fr = &fetchResult{
Expand All @@ -327,7 +330,7 @@ func checkForPath(ctx context.Context, db *postgres.DB, fullPath, modulePath, re
err: err,
}
if errors.Is(err, derrors.NotFound) {
fr.status = http.StatusNoContent
fr.status = statusNotFoundInVersionMap
}
return fr
}
Expand All @@ -353,11 +356,11 @@ func checkForPath(ctx context.Context, db *postgres.DB, fullPath, modulePath, re
return fr
default:
// The module was marked for reprocessing by the worker.
// Return http.StatusNoContent here, so that the tasks gets enqueued
// Return statusNotFoundInVersionMap here, so that the tasks gets enqueued
// to frontend tasks, and we don't return a result to the user until
// that is complete.
if fr.status >= derrors.ToStatus(derrors.ReprocessStatusOK) {
fr.status = http.StatusNoContent
fr.status = statusNotFoundInVersionMap
}
// All remaining non-200 statuses will be in the 40x range.
// In that case, just return a not found error.
Expand Down

0 comments on commit f37c9e1

Please sign in to comment.