7179: Most of golint suggested updates are made. Some names such as "never_delete...
authorradhika <radhika@curoverse.com>
Mon, 14 Sep 2015 14:31:47 +0000 (10:31 -0400)
committerradhika <radhika@curoverse.com>
Mon, 14 Sep 2015 14:31:47 +0000 (10:31 -0400)
Also suggestions such as "replace errors.New(fmt.Sprintf(...)) with fmt.Errorf(...)" are ignored.

services/keepstore/logging_router.go
services/keepstore/pull_worker.go
services/keepstore/pull_worker_integration_test.go
services/keepstore/pull_worker_test.go
services/keepstore/trash_worker.go
services/keepstore/volume_test.go
services/keepstore/volume_unix.go

index b622d1d3eefd2eb3bedfb5e4260976cb2075f85a..47bb6d77179f226dc20d84a525fa5a0de3a5f7af 100644 (file)
@@ -11,6 +11,7 @@ import (
        "time"
 )
 
+// LoggingResponseWriter has anonymous fields ResponseWriter and ResponseBody
 type LoggingResponseWriter struct {
        Status int
        Length int
@@ -18,6 +19,7 @@ type LoggingResponseWriter struct {
        ResponseBody string
 }
 
+// WriteHeader writes header to ResponseWriter
 func (loggingWriter *LoggingResponseWriter) WriteHeader(code int) {
        loggingWriter.Status = code
        loggingWriter.ResponseWriter.WriteHeader(code)
@@ -31,10 +33,12 @@ func (loggingWriter *LoggingResponseWriter) Write(data []byte) (int, error) {
        return loggingWriter.ResponseWriter.Write(data)
 }
 
+// LoggingRESTRouter is used to add logging capabilities to mux.Router
 type LoggingRESTRouter struct {
        router *mux.Router
 }
 
+// MakeLoggingRESTRouter initializes LoggingRESTRouter
 func MakeLoggingRESTRouter() *LoggingRESTRouter {
        router := MakeRESTRouter()
        return (&LoggingRESTRouter{router})
index acf861119f47fd1b765bcad461d826c369151968..9f0b96fa35b1c32af4b06be92ea02632547ef249 100644 (file)
@@ -11,19 +11,18 @@ import (
        "time"
 )
 
-/*
-       Keepstore initiates pull worker channel goroutine.
-       The channel will process pull list.
-               For each (next) pull request:
-                       For each locator listed, execute Pull on the server(s) listed
-                       Skip the rest of the servers if no errors
-               Repeat
-*/
+// RunPullWorker is used by Keepstore to initiate pull worker channel goroutine.
+//     The channel will process pull list.
+//             For each (next) pull request:
+//                     For each locator listed, execute Pull on the server(s) listed
+//                     Skip the rest of the servers if no errors
+//             Repeat
+//
 func RunPullWorker(pullq *WorkQueue, keepClient *keepclient.KeepClient) {
        nextItem := pullq.NextItem
        for item := range nextItem {
                pullRequest := item.(PullRequest)
-               err := PullItemAndProcess(item.(PullRequest), GenerateRandomApiToken(), keepClient)
+               err := PullItemAndProcess(item.(PullRequest), GenerateRandomAPIToken(), keepClient)
                pullq.DoneItem <- struct{}{}
                if err == nil {
                        log.Printf("Pull %s success", pullRequest)
@@ -33,25 +32,25 @@ func RunPullWorker(pullq *WorkQueue, keepClient *keepclient.KeepClient) {
        }
 }
 
-/*
-       For each Pull request:
-               Generate a random API token.
-               Generate a permission signature using this token, timestamp ~60 seconds in the future, and desired block hash.
-               Using this token & signature, retrieve the given block.
-               Write to storage
-*/
+// PullItemAndProcess pulls items from PullQueue and processes them.
+//     For each Pull request:
+//             Generate a random API token.
+//             Generate a permission signature using this token, timestamp ~60 seconds in the future, and desired block hash.
+//             Using this token & signature, retrieve the given block.
+//             Write to storage
+//
 func PullItemAndProcess(pullRequest PullRequest, token string, keepClient *keepclient.KeepClient) (err error) {
        keepClient.Arvados.ApiToken = token
 
-       service_roots := make(map[string]string)
+       serviceRoots := make(map[string]string)
        for _, addr := range pullRequest.Servers {
-               service_roots[addr] = addr
+               serviceRoots[addr] = addr
        }
-       keepClient.SetServiceRoots(service_roots, nil, nil)
+       keepClient.SetServiceRoots(serviceRoots, nil, nil)
 
        // Generate signature with a random token
-       expires_at := time.Now().Add(60 * time.Second)
-       signedLocator := SignLocator(pullRequest.Locator, token, expires_at)
+       expiresAt := time.Now().Add(60 * time.Second)
+       signedLocator := SignLocator(pullRequest.Locator, token, expiresAt)
 
        reader, contentLen, _, err := GetContent(signedLocator, keepClient)
        if err != nil {
@@ -62,16 +61,16 @@ func PullItemAndProcess(pullRequest PullRequest, token string, keepClient *keepc
        }
        defer reader.Close()
 
-       read_content, err := ioutil.ReadAll(reader)
+       readContent, err := ioutil.ReadAll(reader)
        if err != nil {
                return err
        }
 
-       if (read_content == nil) || (int64(len(read_content)) != contentLen) {
+       if (readContent == nil) || (int64(len(readContent)) != contentLen) {
                return errors.New(fmt.Sprintf("Content not found for: %s", signedLocator))
        }
 
-       err = PutContent(read_content, pullRequest.Locator)
+       err = PutContent(readContent, pullRequest.Locator)
        return
 }
 
@@ -82,13 +81,14 @@ var GetContent = func(signedLocator string, keepClient *keepclient.KeepClient) (
        return reader, blocklen, url, err
 }
 
-const ALPHA_NUMERIC = "0123456789abcdefghijklmnopqrstuvwxyz"
+const alphaNumeric = "0123456789abcdefghijklmnopqrstuvwxyz"
 
-func GenerateRandomApiToken() string {
+// GenerateRandomAPIToken generates a random api token
+func GenerateRandomAPIToken() string {
        var bytes = make([]byte, 36)
        rand.Read(bytes)
        for i, b := range bytes {
-               bytes[i] = ALPHA_NUMERIC[b%byte(len(ALPHA_NUMERIC))]
+               bytes[i] = alphaNumeric[b%byte(len(alphaNumeric))]
        }
        return (string(bytes))
 }
index 3e57407369c0dccf8216e3e0835820c0bc419b55..e0bad0045af34d8067591fa723e9229be093790c 100644 (file)
@@ -128,7 +128,7 @@ func performPullWorkerIntegrationTest(testData PullWorkIntegrationTestData, pull
                return rdr, int64(len(testData.Content)), "", nil
        }
 
-       keepClient.Arvados.ApiToken = GenerateRandomApiToken()
+       keepClient.Arvados.ApiToken = GenerateRandomAPIToken()
        err := PullItemAndProcess(pullRequest, keepClient.Arvados.ApiToken, keepClient)
 
        if len(testData.GetError) > 0 {
index 37d83b32802af1432bf7ed8f2af5826a3d757914..ed0d24ae8cb476459bb05f17aa48667d2951a8c4 100644 (file)
@@ -37,7 +37,7 @@ func (s *PullWorkerTestSuite) SetUpTest(c *C) {
 
        // When a new pull request arrives, the old one will be overwritten.
        // This behavior is verified using these two maps in the
-       // "TestPullWorker_pull_list_with_two_items_latest_replacing_old"
+       // "TestPullWorkerPullList_with_two_items_latest_replacing_old"
        testPullLists = make(map[string]string)
 }
 
@@ -53,7 +53,7 @@ func RunTestPullWorker(c *C) {
        go RunPullWorker(pullq, keepClient)
 }
 
-var first_pull_list = []byte(`[
+var firstPullList = []byte(`[
                {
                        "locator":"acbd18db4cc2f85cedef654fccc4a4d8+3",
                        "servers":[
@@ -68,7 +68,7 @@ var first_pull_list = []byte(`[
                }
        ]`)
 
-var second_pull_list = []byte(`[
+var secondPullList = []byte(`[
                {
                        "locator":"73feffa4b7f6bb68e44cf984c85f6e88+3",
                        "servers":[
@@ -79,44 +79,44 @@ var second_pull_list = []byte(`[
        ]`)
 
 type PullWorkerTestData struct {
-       name          string
-       req           RequestTester
-       response_code int
-       response_body string
-       read_content  string
-       read_error    bool
-       put_error     bool
+       name         string
+       req          RequestTester
+       responseCode int
+       responseBody string
+       readContent  string
+       readError    bool
+       putError     bool
 }
 
-func (s *PullWorkerTestSuite) TestPullWorker_pull_list_with_two_locators(c *C) {
+func (s *PullWorkerTestSuite) TestPullWorkerPullList_with_two_locators(c *C) {
        defer teardown()
 
        data_manager_token = "DATA MANAGER TOKEN"
        testData := PullWorkerTestData{
-               name:          "TestPullWorker_pull_list_with_two_locators",
-               req:           RequestTester{"/pull", data_manager_token, "PUT", first_pull_list},
-               response_code: http.StatusOK,
-               response_body: "Received 2 pull requests\n",
-               read_content:  "hello",
-               read_error:    false,
-               put_error:     false,
+               name:         "TestPullWorkerPullList_with_two_locators",
+               req:          RequestTester{"/pull", data_manager_token, "PUT", firstPullList},
+               responseCode: http.StatusOK,
+               responseBody: "Received 2 pull requests\n",
+               readContent:  "hello",
+               readError:    false,
+               putError:     false,
        }
 
        performTest(testData, c)
 }
 
-func (s *PullWorkerTestSuite) TestPullWorker_pull_list_with_one_locator(c *C) {
+func (s *PullWorkerTestSuite) TestPullWorkerPullList_with_one_locator(c *C) {
        defer teardown()
 
        data_manager_token = "DATA MANAGER TOKEN"
        testData := PullWorkerTestData{
-               name:          "TestPullWorker_pull_list_with_one_locator",
-               req:           RequestTester{"/pull", data_manager_token, "PUT", second_pull_list},
-               response_code: http.StatusOK,
-               response_body: "Received 1 pull requests\n",
-               read_content:  "hola",
-               read_error:    false,
-               put_error:     false,
+               name:         "TestPullWorkerPullList_with_one_locator",
+               req:          RequestTester{"/pull", data_manager_token, "PUT", secondPullList},
+               responseCode: http.StatusOK,
+               responseBody: "Received 1 pull requests\n",
+               readContent:  "hola",
+               readError:    false,
+               putError:     false,
        }
 
        performTest(testData, c)
@@ -127,13 +127,13 @@ func (s *PullWorkerTestSuite) TestPullWorker_error_on_get_one_locator(c *C) {
 
        data_manager_token = "DATA MANAGER TOKEN"
        testData := PullWorkerTestData{
-               name:          "TestPullWorker_error_on_get_one_locator",
-               req:           RequestTester{"/pull", data_manager_token, "PUT", second_pull_list},
-               response_code: http.StatusOK,
-               response_body: "Received 1 pull requests\n",
-               read_content:  "unused",
-               read_error:    true,
-               put_error:     false,
+               name:         "TestPullWorker_error_on_get_one_locator",
+               req:          RequestTester{"/pull", data_manager_token, "PUT", secondPullList},
+               responseCode: http.StatusOK,
+               responseBody: "Received 1 pull requests\n",
+               readContent:  "unused",
+               readError:    true,
+               putError:     false,
        }
 
        performTest(testData, c)
@@ -144,13 +144,13 @@ func (s *PullWorkerTestSuite) TestPullWorker_error_on_get_two_locators(c *C) {
 
        data_manager_token = "DATA MANAGER TOKEN"
        testData := PullWorkerTestData{
-               name:          "TestPullWorker_error_on_get_two_locators",
-               req:           RequestTester{"/pull", data_manager_token, "PUT", first_pull_list},
-               response_code: http.StatusOK,
-               response_body: "Received 2 pull requests\n",
-               read_content:  "unused",
-               read_error:    true,
-               put_error:     false,
+               name:         "TestPullWorker_error_on_get_two_locators",
+               req:          RequestTester{"/pull", data_manager_token, "PUT", firstPullList},
+               responseCode: http.StatusOK,
+               responseBody: "Received 2 pull requests\n",
+               readContent:  "unused",
+               readError:    true,
+               putError:     false,
        }
 
        performTest(testData, c)
@@ -161,13 +161,13 @@ func (s *PullWorkerTestSuite) TestPullWorker_error_on_put_one_locator(c *C) {
 
        data_manager_token = "DATA MANAGER TOKEN"
        testData := PullWorkerTestData{
-               name:          "TestPullWorker_error_on_put_one_locator",
-               req:           RequestTester{"/pull", data_manager_token, "PUT", second_pull_list},
-               response_code: http.StatusOK,
-               response_body: "Received 1 pull requests\n",
-               read_content:  "hello hello",
-               read_error:    false,
-               put_error:     true,
+               name:         "TestPullWorker_error_on_put_one_locator",
+               req:          RequestTester{"/pull", data_manager_token, "PUT", secondPullList},
+               responseCode: http.StatusOK,
+               responseBody: "Received 1 pull requests\n",
+               readContent:  "hello hello",
+               readError:    false,
+               putError:     true,
        }
 
        performTest(testData, c)
@@ -178,13 +178,13 @@ func (s *PullWorkerTestSuite) TestPullWorker_error_on_put_two_locators(c *C) {
 
        data_manager_token = "DATA MANAGER TOKEN"
        testData := PullWorkerTestData{
-               name:          "TestPullWorker_error_on_put_two_locators",
-               req:           RequestTester{"/pull", data_manager_token, "PUT", first_pull_list},
-               response_code: http.StatusOK,
-               response_body: "Received 2 pull requests\n",
-               read_content:  "hello again",
-               read_error:    false,
-               put_error:     true,
+               name:         "TestPullWorker_error_on_put_two_locators",
+               req:          RequestTester{"/pull", data_manager_token, "PUT", firstPullList},
+               responseCode: http.StatusOK,
+               responseBody: "Received 2 pull requests\n",
+               readContent:  "hello again",
+               readError:    false,
+               putError:     true,
        }
 
        performTest(testData, c)
@@ -194,7 +194,7 @@ func (s *PullWorkerTestSuite) TestPullWorker_error_on_put_two_locators(c *C) {
 // is used to check that behavior by first putting an item on the queue,
 // and then performing the test. Thus the "testPullLists" has two entries;
 // however, processedPullLists will see only the newest item in the list.
-func (s *PullWorkerTestSuite) TestPullWorker_pull_list_with_two_items_latest_replacing_old(c *C) {
+func (s *PullWorkerTestSuite) TestPullWorkerPullList_with_two_items_latest_replacing_old(c *C) {
        defer teardown()
 
        var firstInput = []int{1}
@@ -204,13 +204,13 @@ func (s *PullWorkerTestSuite) TestPullWorker_pull_list_with_two_items_latest_rep
 
        data_manager_token = "DATA MANAGER TOKEN"
        testData := PullWorkerTestData{
-               name:          "TestPullWorker_pull_list_with_two_items_latest_replacing_old",
-               req:           RequestTester{"/pull", data_manager_token, "PUT", second_pull_list},
-               response_code: http.StatusOK,
-               response_body: "Received 1 pull requests\n",
-               read_content:  "hola de nuevo",
-               read_error:    false,
-               put_error:     false,
+               name:         "TestPullWorkerPullList_with_two_items_latest_replacing_old",
+               req:          RequestTester{"/pull", data_manager_token, "PUT", secondPullList},
+               responseCode: http.StatusOK,
+               responseBody: "Received 1 pull requests\n",
+               readContent:  "hola de nuevo",
+               readError:    false,
+               putError:     false,
        }
 
        performTest(testData, c)
@@ -223,13 +223,13 @@ func (s *PullWorkerTestSuite) TestPullWorker_invalid_data_manager_token(c *C) {
        data_manager_token = "DATA MANAGER TOKEN"
 
        testData := PullWorkerTestData{
-               name:          "TestPullWorker_pull_list_with_two_locators",
-               req:           RequestTester{"/pull", "invalid_data_manager_token", "PUT", first_pull_list},
-               response_code: http.StatusUnauthorized,
-               response_body: "Unauthorized\n",
-               read_content:  "hello",
-               read_error:    false,
-               put_error:     false,
+               name:         "TestPullWorkerPullList_with_two_locators",
+               req:          RequestTester{"/pull", "invalid_data_manager_token", "PUT", firstPullList},
+               responseCode: http.StatusUnauthorized,
+               responseBody: "Unauthorized\n",
+               readContent:  "hello",
+               readError:    false,
+               putError:     false,
        }
 
        performTest(testData, c)
@@ -243,7 +243,7 @@ func performTest(testData PullWorkerTestData, c *C) {
        defer pullq.Close()
 
        currentTestData = testData
-       testPullLists[testData.name] = testData.response_body
+       testPullLists[testData.name] = testData.responseBody
 
        processedPullLists := make(map[string]string)
 
@@ -253,53 +253,51 @@ func performTest(testData PullWorkerTestData, c *C) {
        }(GetContent)
        GetContent = func(signedLocator string, keepClient *keepclient.KeepClient) (reader io.ReadCloser, contentLength int64, url string, err error) {
                c.Assert(getStatusItem("PullQueue", "InProgress"), Equals, float64(1))
-               processedPullLists[testData.name] = testData.response_body
-               if testData.read_error {
+               processedPullLists[testData.name] = testData.responseBody
+               if testData.readError {
                        err = errors.New("Error getting data")
                        readError = err
                        return nil, 0, "", err
-               } else {
-                       readContent = testData.read_content
-                       cb := &ClosingBuffer{bytes.NewBufferString(testData.read_content)}
-                       var rc io.ReadCloser
-                       rc = cb
-                       return rc, int64(len(testData.read_content)), "", nil
                }
+               readContent = testData.readContent
+               cb := &ClosingBuffer{bytes.NewBufferString(testData.readContent)}
+               var rc io.ReadCloser
+               rc = cb
+               return rc, int64(len(testData.readContent)), "", nil
        }
 
        // Override PutContent to mock PutBlock functionality
        defer func(orig func([]byte, string) error) { PutContent = orig }(PutContent)
        PutContent = func(content []byte, locator string) (err error) {
-               if testData.put_error {
+               if testData.putError {
                        err = errors.New("Error putting data")
                        putError = err
                        return err
-               } else {
-                       putContent = content
-                       return nil
                }
+               putContent = content
+               return nil
        }
 
        c.Assert(getStatusItem("PullQueue", "InProgress"), Equals, float64(0))
        c.Assert(getStatusItem("PullQueue", "Queued"), Equals, float64(0))
 
        response := IssueRequest(&testData.req)
-       c.Assert(response.Code, Equals, testData.response_code)
-       c.Assert(response.Body.String(), Equals, testData.response_body)
+       c.Assert(response.Code, Equals, testData.responseCode)
+       c.Assert(response.Body.String(), Equals, testData.responseBody)
 
        expectEqualWithin(c, time.Second, 0, func() interface{} {
                st := pullq.Status()
                return st.InProgress + st.Queued
        })
 
-       if testData.name == "TestPullWorker_pull_list_with_two_items_latest_replacing_old" {
+       if testData.name == "TestPullWorkerPullList_with_two_items_latest_replacing_old" {
                c.Assert(len(testPullLists), Equals, 2)
                c.Assert(len(processedPullLists), Equals, 1)
                c.Assert(testPullLists["Added_before_actual_test_item"], NotNil)
-               c.Assert(testPullLists["TestPullWorker_pull_list_with_two_items_latest_replacing_old"], NotNil)
-               c.Assert(processedPullLists["TestPullWorker_pull_list_with_two_items_latest_replacing_old"], NotNil)
+               c.Assert(testPullLists["TestPullWorkerPullList_with_two_items_latest_replacing_old"], NotNil)
+               c.Assert(processedPullLists["TestPullWorkerPullList_with_two_items_latest_replacing_old"], NotNil)
        } else {
-               if testData.response_code == http.StatusOK {
+               if testData.responseCode == http.StatusOK {
                        c.Assert(len(testPullLists), Equals, 1)
                        c.Assert(len(processedPullLists), Equals, 1)
                        c.Assert(testPullLists[testData.name], NotNil)
@@ -309,16 +307,16 @@ func performTest(testData PullWorkerTestData, c *C) {
                }
        }
 
-       if testData.read_error {
+       if testData.readError {
                c.Assert(readError, NotNil)
-       } else if testData.response_code == http.StatusOK {
+       } else if testData.responseCode == http.StatusOK {
                c.Assert(readError, IsNil)
-               c.Assert(readContent, Equals, testData.read_content)
-               if testData.put_error {
+               c.Assert(readContent, Equals, testData.readContent)
+               if testData.putError {
                        c.Assert(putError, NotNil)
                } else {
                        c.Assert(putError, IsNil)
-                       c.Assert(string(putContent), Equals, testData.read_content)
+                       c.Assert(string(putContent), Equals, testData.readContent)
                }
        }
 
index 8f78658c3a7496473c2d81a7f0d7b13213ef9d5f..7e19ba41c5d8250f0d678ea0c107972e3c077374 100644 (file)
@@ -6,14 +6,12 @@ import (
        "time"
 )
 
-/*
-       Keepstore initiates trash worker channel goroutine.
-       The channel will process trash list.
-               For each (next) trash request:
-      Delete the block indicated by the trash request Locator
-               Repeat
-*/
-
+// RunTrashWorker is used by Keepstore to initiate trash worker channel goroutine.
+//     The channel will process trash list.
+//             For each (next) trash request:
+//      Delete the block indicated by the trash request Locator
+//             Repeat
+//
 func RunTrashWorker(trashq *WorkQueue) {
        for item := range trashq.NextItem {
                trashRequest := item.(TrashRequest)
index 290722557598fd64118feac5eb567192cdbc08c2..5467ed541b3dc90974e3d70ea531971b48f70341 100644 (file)
@@ -52,7 +52,7 @@ type MockVolume struct {
        // channel unblocks all operations. By default, Gate is a
        // closed channel, so all operations proceed without
        // blocking. See trash_worker_test.go for an example.
-       Gate   chan struct{}
+       Gate chan struct{}
 
        called map[string]int
        mutex  sync.Mutex
@@ -78,11 +78,11 @@ func CreateMockVolume() *MockVolume {
 func (v *MockVolume) CallCount(method string) int {
        v.mutex.Lock()
        defer v.mutex.Unlock()
-       if c, ok := v.called[method]; !ok {
+       c, ok := v.called[method]
+       if !ok {
                return 0
-       } else {
-               return c
        }
+       return c
 }
 
 func (v *MockVolume) gotCall(method string) {
index 12fcef8c4f2ced4087dc1b3c777fedf9401ec5c6..f4b49710e9192deabb2119d72ccbb389edcbe7bc 100644 (file)
@@ -26,6 +26,7 @@ type UnixVolume struct {
        readonly bool
 }
 
+// Touch sets the timestamp for the given locator to the current time
 func (v *UnixVolume) Touch(loc string) error {
        if v.readonly {
                return MethodDisabledError
@@ -49,13 +50,14 @@ func (v *UnixVolume) Touch(loc string) error {
        return syscall.Utime(p, &utime)
 }
 
+// Mtime returns the stored timestamp for the given locator.
 func (v *UnixVolume) Mtime(loc string) (time.Time, error) {
        p := v.blockPath(loc)
-       if fi, err := os.Stat(p); err != nil {
+       fi, err := os.Stat(p)
+       if err != nil {
                return time.Time{}, err
-       } else {
-               return fi.ModTime(), nil
        }
+       return fi.ModTime(), nil
 }
 
 // Lock the locker (if one is in use), open the file for reading, and
@@ -288,6 +290,7 @@ func (v *UnixVolume) IndexTo(prefix string, w io.Writer) error {
        }
 }
 
+// Delete deletes the block data from the unix storage
 func (v *UnixVolume) Delete(loc string) error {
        // Touch() must be called before calling Write() on a block.  Touch()
        // also uses lockfile().  This avoids a race condition between Write()
@@ -391,6 +394,7 @@ func (v *UnixVolume) String() string {
        return fmt.Sprintf("[UnixVolume %s]", v.root)
 }
 
+// Writable returns false if all future Put, Mtime, and Delete calls are expected to fail.
 func (v *UnixVolume) Writable() bool {
        return !v.readonly
 }