From 56bb5d1a38e5bdba962763aaf601bfdadb865f72 Mon Sep 17 00:00:00 2001 From: Gina White Date: Sun, 23 Aug 2015 07:00:20 -0700 Subject: [PATCH] Address TODO by moving CAMLI_DEBUG checks into env I also wonder if we want to take a more comprehensive approach to environment variables, perhaps populating a struct at startup that other code can consult later. But it might be too soon for that kind of thing in this project. Change-Id: I65a34622bf906c1256ceb357ba983bc5acd6b887 --- TODO | 3 --- pkg/blobserver/diskpacked/diskpacked_test.go | 3 ++- pkg/blobserver/diskpacked/reindex.go | 9 ++++----- pkg/client/upload.go | 9 ++++----- pkg/env/env.go | 19 ++++++++++++++++++- pkg/schema/filereader.go | 3 ++- 6 files changed, 30 insertions(+), 16 deletions(-) diff --git a/TODO b/TODO index a3d247a70..3eeebf108 100644 --- a/TODO +++ b/TODO @@ -11,9 +11,6 @@ Offline list: exit status 2 make: *** [fmt] Error 1 --- env package to put stuff CAMLI_DEBUG checks? Also, change all: - -var debug = os.Getenv("CAMLI_DEBUG") != "" - +var debug, _ = strconv.ParseBool(os.Getenv("CAMLI_DEBUG")) -- add HTTP handler for blobstreamer. stream a tar file? where to put continuation token? special file after each tar entry? special file diff --git a/pkg/blobserver/diskpacked/diskpacked_test.go b/pkg/blobserver/diskpacked/diskpacked_test.go index 5661aff9f..3a06f5244 100644 --- a/pkg/blobserver/diskpacked/diskpacked_test.go +++ b/pkg/blobserver/diskpacked/diskpacked_test.go @@ -28,6 +28,7 @@ import ( "camlistore.org/pkg/blob" "camlistore.org/pkg/blobserver" "camlistore.org/pkg/blobserver/storagetest" + "camlistore.org/pkg/env" "camlistore.org/pkg/jsonconfig" "camlistore.org/pkg/sorted" "camlistore.org/pkg/test" @@ -56,7 +57,7 @@ func newTempDiskpackedWithIndex(t *testing.T, indexConf jsonconfig.Obj) (sto blo } return s, func() { s.Close() - if camliDebug { + if env.IsDebug() { t.Logf("CAMLI_DEBUG set, skipping cleanup of dir %q", dir) } else { os.RemoveAll(dir) diff --git a/pkg/blobserver/diskpacked/reindex.go b/pkg/blobserver/diskpacked/reindex.go index 51e974c8e..5b93960cf 100644 --- a/pkg/blobserver/diskpacked/reindex.go +++ b/pkg/blobserver/diskpacked/reindex.go @@ -28,6 +28,7 @@ import ( "camlistore.org/pkg/blob" "camlistore.org/pkg/context" + "camlistore.org/pkg/env" "camlistore.org/pkg/jsonconfig" "camlistore.org/pkg/sorted" @@ -37,8 +38,6 @@ import ( _ "camlistore.org/pkg/sorted/sqlite" ) -var camliDebug, _ = strconv.ParseBool(os.Getenv("CAMLI_DEBUG")) - // Reindex rewrites the index files of the diskpacked .pack files func Reindex(root string, overwrite bool, indexConf jsonconfig.Obj) (err error) { // there is newStorage, but that may open a file for writing @@ -84,12 +83,12 @@ func (s *storage) reindexOne(ctx *context.Context, index sorted.KeyValue, overwr allOk := true // TODO(tgulacsi): proper verbose from context - verbose := camliDebug + verbose := env.IsDebug() misses := make(map[blob.Ref]string, 8) err := s.walkPack(verbose, packID, func(packID int, ref blob.Ref, offset int64, size uint32) error { if !ref.Valid() { - if camliDebug { + if verbose { log.Printf("found deleted blob in %d at %d with size %d", packID, offset, size) } return nil @@ -145,7 +144,7 @@ func (s *storage) Walk(ctx *context.Context, walker func(packID int, ref blob.Ref, offset int64, size uint32) error) error { // TODO(tgulacsi): proper verbose flag from context - verbose := camliDebug + verbose := env.IsDebug() for i := 0; i >= 0; i++ { fh, err := os.Open(s.filename(i)) diff --git a/pkg/client/upload.go b/pkg/client/upload.go index 6bae61528..d27d23113 100644 --- a/pkg/client/upload.go +++ b/pkg/client/upload.go @@ -34,11 +34,10 @@ import ( "camlistore.org/pkg/blobserver" "camlistore.org/pkg/blobserver/protocol" "camlistore.org/pkg/constants" + "camlistore.org/pkg/env" "camlistore.org/pkg/httputil" ) -var debugUploads = os.Getenv("CAMLI_DEBUG_UPLOADS") != "" - // multipartOverhead is how many extra bytes mime/multipart's // Writer adds around content var multipartOverhead = calculateMultipartOverhead() @@ -236,7 +235,7 @@ func (c *Client) doSomeStats() { } c.pendStatMu.Unlock() - if debugUploads { + if env.DebugUploads() { println("doing stat batch of", len(batch)) } @@ -405,7 +404,7 @@ func (c *Client) Upload(h *UploadHandle) (*PutResult, error) { c.haveCache.NoteBlobExists(sbr.Ref, uint32(sbr.Size)) } _, serverHasIt := stat.HaveMap[blobrefStr] - if debugUploads { + if env.DebugUploads() { log.Printf("HTTP Stat(%s) = %v", blobrefStr, serverHasIt) } if !h.Vivify && serverHasIt { @@ -422,7 +421,7 @@ func (c *Client) Upload(h *UploadHandle) (*PutResult, error) { } } - if debugUploads { + if env.DebugUploads() { log.Printf("Uploading: %s (%d bytes)", blobrefStr, bodySize) } diff --git a/pkg/env/env.go b/pkg/env/env.go index 08796e8e6..79befcaf1 100644 --- a/pkg/env/env.go +++ b/pkg/env/env.go @@ -19,14 +19,25 @@ package env import ( "os" + "strconv" "sync" "google.golang.org/cloud/compute/metadata" ) +// IsDebug reports whether this is a debug environment. +func IsDebug() bool { + return isDebug +} + +// DebugUploads reports whether this is a debug environment for uploads. +func DebugUploads() bool { + return isDebugUploads +} + // IsDev reports whether this is a development server environment (devcam server). func IsDev() bool { - return os.Getenv("CAMLI_DEV_CAMLI_ROOT") != "" + return isDev } // OsGCE reports whether this process is running in a Google Compute @@ -50,3 +61,9 @@ func detectGCE() { v, _ := metadata.InstanceAttributeValue("camlistore-config-dir") isGCE = v != "" } + +var ( + isDev = os.Getenv("CAMLI_DEV_CAMLI_ROOT") != "" + isDebug, _ = strconv.ParseBool(os.Getenv("CAMLI_DEBUG")) + isDebugUploads, _ = strconv.ParseBool(os.Getenv("CAMLI_DEBUG_UPLOADS")) +) diff --git a/pkg/schema/filereader.go b/pkg/schema/filereader.go index 3cfbe8c35..cb805de48 100644 --- a/pkg/schema/filereader.go +++ b/pkg/schema/filereader.go @@ -27,6 +27,7 @@ import ( "time" "camlistore.org/pkg/blob" + "camlistore.org/pkg/env" "camlistore.org/pkg/singleflight" "camlistore.org/pkg/syncutil" "camlistore.org/pkg/types" @@ -307,7 +308,7 @@ func (fr *FileReader) getSuperset(br blob.Ref) (*superset, error) { return ssi.(*superset), nil } -var debug = os.Getenv("CAMLI_DEBUG") != "" +var debug = env.IsDebug() // readerForOffset returns a ReadCloser that reads some number of bytes and then EOF // from the provided offset. Seeing EOF doesn't mean the end of the whole file; just the