LogDog: Fix archived log stream read errors.

common/gcloud/gs/gs.go

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 package gs
 
 import (
         "fmt"
         "io"
         "net/http"
@@ skipping 19 matching lines @@
 )
 
 // Client abstracts funcitonality to connect with and use Google Storage from
 // the actual Google Storage client.
 //
 // Non-production implementations are used primarily for testing.
 type Client interface {
         io.Closer
 
         // NewReader instantiates a new Reader instance for the named bucket/path.
-        NewReader(p Path, o Options) (io.ReadCloser, error)
+        //
+        // If the supplied offset is <0, it will default to 0. If the supplied length
+        // is <0, no upper byte bound will be set.

[nodir, 2016/04/19 23:41:16]

why not accept uint64 or panic? if someone passes a negative value, it sounds
like a bug in the caller code.
if you use uint64, you wouldn't have to do type conversion in client code

[dnj, 2016/04/20 00:45:07]

We generally don't do uint64 in our code, even for positive-only values. There
was a long discussion in the past about this, so I'll omit the specifics. Other
APIs (including Cloud Storage) also generally don't use uint64.

[nodir, 2016/04/20 16:42:57]

ok, why not panic? If there is a negative value, it is a bug in your code. Don't
you want to find bugs?

[dnj, 2016/04/20 17:07:25]

Sure, I'll panic if offset is <0.

+        NewReader(p Path, offset int64, length int64) (io.ReadCloser, error)

[nodir, 2016/04/19 23:41:16]

nit: offset, length int64

[dnj, 2016/04/20 00:45:07]

Done.

 
         // NewWriter instantiates a new Writer instance for the named bucket/path.
         NewWriter(p Path) (Writer, error)
 
         // Delete deletes the object at the specified path.
         //
         // If the object does not exist, it is considered a success.
         Delete(p Path) error
 
         // Rename renames an object from one path to another.
         //
         // NOTE: The object should be removed from its original path, but current
         // implementation uses two operations (Copy + Delete), so it may
         // occasionally fail.
         Rename(src, dst Path) error
 }
 
-// Options are the set of extra options to apply to the Google Storage request.
-type Options struct {
-        // From is the range request starting index. If >0, the beginning of the
-        // range request will be set.
-        From int64
-        // To is the range request ending index. If >0, the end of the
-        // range request will be set.
-        //
-        // If no From index is set, this will result in a request indexed from the end
-        // of the object.
-        To int64
-}
-
 // prodGSObject is an implementation of Client interface using the production
 // Google Storage client.
 type prodClient struct {
         context.Context
 
         // rt is the RoundTripper to use, or nil for the cloud service default.
         rt http.RoundTripper
         // baseClient is a basic Google Storage client instance. It is used for
         // operations that don't need custom header injections.
         baseClient *gs.Client
@@ skipping 28 matching lines @@
         }
 
         return &prodWriter{
                 Context: c,
                 client:  c,
                 bucket:  bucket,
                 relpath: filename,
         }, nil
 }
 
-func (c *prodClient) NewReader(p Path, o Options) (io.ReadCloser, error) {
-        if o.From < 0 {
-                o.From = 0
-        }
-        if o.To <= 0 {
-                o.To = -1
+func (c *prodClient) NewReader(p Path, offset int64, length int64) (io.ReadCloser, error) {

[nodir, 2016/04/19 23:41:16]

nit: offset, length int64

[dnj, 2016/04/20 00:45:07]

Done.

+        if offset < 0 {
+                offset = 0
         }
 
         obj, err := c.handleForPath(p)
         if err != nil {
                 return nil, err
         }
-        return obj.NewRangeReader(c, o.From, o.To)
+        return obj.NewRangeReader(c, offset, length)
 }
 
 func (c *prodClient) Rename(src, dst Path) error {
         srcObj, err := c.handleForPath(src)
         if err != nil {
                 return fmt.Errorf("invalid source path: %s", err)
         }
 
         dstObj, err := c.handleForPath(dst)
         if err != nil {
@@ skipping 100 matching lines @@
         // The storage library doesn't return gs.ErrObjectNotExist when Delete
         // returns a 404. Catch that explicitly.
         if t, ok := err.(*googleapi.Error); ok {
                 switch t.Code {
                 case http.StatusNotFound:
                         return true
                 }
         }
         return false
 }