[ARVADOS] updated: 04a40b8028387c4b8899d6b5b0a55d349ddab04b
git at public.curoverse.com
git at public.curoverse.com
Mon Sep 14 10:34:52 EDT 2015
Summary of changes:
services/keepstore/logging_router.go | 4 +
services/keepstore/pull_worker.go | 54 +++---
services/keepstore/pull_worker_integration_test.go | 2 +-
services/keepstore/pull_worker_test.go | 184 ++++++++++-----------
services/keepstore/trash_worker.go | 14 +-
services/keepstore/volume_test.go | 8 +-
services/keepstore/volume_unix.go | 10 +-
7 files changed, 140 insertions(+), 136 deletions(-)
via 04a40b8028387c4b8899d6b5b0a55d349ddab04b (commit)
from 63f2191f07851bc66756de518644f2259b938d88 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
commit 04a40b8028387c4b8899d6b5b0a55d349ddab04b
Author: radhika <radhika at curoverse.com>
Date: Mon Sep 14 10:31:47 2015 -0400
7179: Most of golint suggested updates are made. Some names such as "never_delete" are left alone.
Also suggestions such as "replace errors.New(fmt.Sprintf(...)) with fmt.Errorf(...)" are ignored.
diff --git a/services/keepstore/logging_router.go b/services/keepstore/logging_router.go
index b622d1d..47bb6d7 100644
--- a/services/keepstore/logging_router.go
+++ b/services/keepstore/logging_router.go
@@ -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})
diff --git a/services/keepstore/pull_worker.go b/services/keepstore/pull_worker.go
index acf8611..9f0b96f 100644
--- a/services/keepstore/pull_worker.go
+++ b/services/keepstore/pull_worker.go
@@ -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))
}
diff --git a/services/keepstore/pull_worker_integration_test.go b/services/keepstore/pull_worker_integration_test.go
index 3e57407..e0bad00 100644
--- a/services/keepstore/pull_worker_integration_test.go
+++ b/services/keepstore/pull_worker_integration_test.go
@@ -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 {
diff --git a/services/keepstore/pull_worker_test.go b/services/keepstore/pull_worker_test.go
index 37d83b3..ed0d24a 100644
--- a/services/keepstore/pull_worker_test.go
+++ b/services/keepstore/pull_worker_test.go
@@ -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)
}
}
diff --git a/services/keepstore/trash_worker.go b/services/keepstore/trash_worker.go
index 8f78658..7e19ba4 100644
--- a/services/keepstore/trash_worker.go
+++ b/services/keepstore/trash_worker.go
@@ -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)
diff --git a/services/keepstore/volume_test.go b/services/keepstore/volume_test.go
index 2907225..5467ed5 100644
--- a/services/keepstore/volume_test.go
+++ b/services/keepstore/volume_test.go
@@ -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) {
diff --git a/services/keepstore/volume_unix.go b/services/keepstore/volume_unix.go
index 12fcef8..f4b4971 100644
--- a/services/keepstore/volume_unix.go
+++ b/services/keepstore/volume_unix.go
@@ -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
}
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list