[pkgsite-metrics] cmd/jobs: simplify upload logic

Jonathan Amsterdam (Gerrit)

unread,

May 27, 2023, 12:12:58 PM5/27/23

to Zvonimir Pavlinovic, Maceo Thompson, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Attention is currently required from: Maceo Thompson, Zvonimir Pavlinovic.

Jonathan Amsterdam would like Zvonimir Pavlinovic and Maceo Thompson to review this change.

cmd/jobs: simplify upload logic

Change-Id: Ia217b376a5de5d7559db74746ae14545eea6242b
---
M cmd/jobs/main.go
1 file changed, 20 insertions(+), 26 deletions(-)

diff --git a/cmd/jobs/main.go b/cmd/jobs/main.go
index e34bafb..aaaa355 100644
--- a/cmd/jobs/main.go
+++ b/cmd/jobs/main.go
@@ -186,19 +186,20 @@
 	}
 
 	// Ask the server to enqueue scan tasks.
-	its, err := identityTokenSource(ctx)
-	if err != nil {
-		return err
-	}
-	url := fmt.Sprintf("%s/analysis/enqueue?binary=%s&user=%s", workerURL, filepath.Base(binaryFile), os.Getenv("USER"))
-	if min >= 0 {
-		url += fmt.Sprintf("&min=%d", min)
-	}
-	body, err := httpGet(ctx, url, its)
-	if err != nil {
-		return err
-	}
-	fmt.Printf("%s\n", body)
+	// its, err := identityTokenSource(ctx)
+	// if err != nil {
+	// 	return err
+	// }
+	// url := fmt.Sprintf("%s/analysis/enqueue?binary=%s&user=%s", workerURL, filepath.Base(binaryFile), os.Getenv("USER"))
+	// if min >= 0 {
+	// 	url += fmt.Sprintf("&min=%d", min)
+	// }
+	// body, err := httpGet(ctx, url, its)
+	// if err != nil {
+	// 	return err
+	// }
+	// fmt.Printf("%s\n", body)
+	_ = min
 	return nil
 }
 
@@ -207,7 +208,6 @@
 // As an optimization, it skips the upload if the file is already on GCS
 // and has the same checksum as the local file.
 func uploadAnalysisBinary(ctx context.Context, binaryFile string) error {
-	var upload bool
 	const bucketName = projectID
 	binaryName := filepath.Base(binaryFile)
 	objectName := path.Join("analysis-binaries", binaryName)
@@ -226,7 +226,6 @@
 	attrs, err := object.Attrs(ctx)
 	if errors.Is(err, storage.ErrObjectNotExist) {
 		fmt.Printf("%s does not exist, uploading\n", object.ObjectName())
-		upload = true
 	} else if err != nil {
 		return err
 	} else if g, w := len(attrs.MD5), md5.Size; g != w {
@@ -236,20 +235,15 @@
 		if err != nil {
 			return err
 		}
-		upload = !bytes.Equal(localMD5, attrs.MD5)
-		if upload {
-			fmt.Printf("binary %s exists on GCS but hashes don't match; uploading\n", binaryName)
-		} else {
+		if bytes.Equal(localMD5, attrs.MD5) {
 			fmt.Printf("%s already on GCS with same checksum; not uploading\n", binaryFile)
+			return nil
+		} else {
+			fmt.Printf("binary %s exists on GCS but hashes don't match; uploading\n", binaryName)
 		}
 	}
-	if upload {
-		if err := copyToGCS(ctx, object, binaryFile); err != nil {
-			return err
-		}
-		fmt.Printf("copied %s to %s\n", binaryFile, object.ObjectName())
-	}
-	return nil
+	fmt.Printf("copying %s to %s\n", binaryFile, object.ObjectName())
+	return copyToGCS(ctx, object, binaryFile)
 }
 
 // fileMD5 computes the MD5 checksum of the given file.

To view, visit change 498855. To unsubscribe, or for help writing mail filters, visit settings.

Zvonimir Pavlinovic (Gerrit)

unread,

May 28, 2023, 7:04:27 AM5/28/23

to Jonathan Amsterdam, goph...@pubsubhelper.golang.org, Gopher Robot, Maceo Thompson, golang-co...@googlegroups.com

Attention is currently required from: Jonathan Amsterdam, Maceo Thompson.

View Change

1 comment:

File cmd/jobs/main.go:
- Patch Set #1, Line 188:

// Ask the server to enqueue scan tasks.

	// its, err := identityTokenSource(ctx)

- ```
	// if err != nil {
```

- ```
	// 	return err
	// }
```


	// url := fmt.Sprintf("%s/analysis/enqueue?binary=%s&user=%s", workerURL, filepath.Base(binaryFile), os.Getenv("USER"))

- ```
	// if min >= 0 {
```

- ```
	// 	url += fmt.Sprintf("&min=%d", min)
```

- ```
	// }
```

- ```
	// body, err := httpGet(ctx, url, its)
```

- ```
	// if err != nil {
```

- ```
	// 	return err
	// }
```

- ```
	// fmt.Printf("%s\n", body)
```

- ```
	_ = min
```
  debugging leftover?

To view, visit change 498855. To unsubscribe, or for help writing mail filters, visit settings.

Jonathan Amsterdam (Gerrit)

unread,

May 30, 2023, 1:41:12 PM5/30/23

to goph...@pubsubhelper.golang.org, Gopher Robot, Zvonimir Pavlinovic, Maceo Thompson, golang-co...@googlegroups.com

Attention is currently required from: Maceo Thompson, Zvonimir Pavlinovic.

View Change

1 comment:

File cmd/jobs/main.go:

Patch Set #1, Line 188:

// Ask the server to enqueue scan tasks.
	// its, err := identityTokenSource(ctx)
	// if err != nil {
	// 	return err
	// }
	// url := fmt.Sprintf("%s/analysis/enqueue?binary=%s&user=%s", workerURL, filepath.Base(binaryFile), os.Getenv("USER"))
	// if min >= 0 {
	// 	url += fmt.Sprintf("&min=%d", min)
	// }
	// body, err := httpGet(ctx, url, its)
	// if err != nil {
	// 	return err
	// }
	// fmt.Printf("%s\n", body)
	_ = min

debugging leftover?

- Actually good code that I commented out for an informal test. Restored.

To view, visit change 498855. To unsubscribe, or for help writing mail filters, visit settings.

Jonathan Amsterdam (Gerrit)

unread,

May 30, 2023, 1:41:59 PM5/30/23

to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Attention is currently required from: Jonathan Amsterdam, Maceo Thompson, Zvonimir Pavlinovic.

Jonathan Amsterdam uploaded patch set #2 to this change.

View Change

The following approvals got outdated and were removed: Run-TryBot+1 by Jonathan Amsterdam, TryBot-Result+1 by Gopher Robot

cmd/jobs: simplify upload logic

Change-Id: Ia217b376a5de5d7559db74746ae14545eea6242b
---
M cmd/jobs/main.go

1 file changed, 6 insertions(+), 13 deletions(-)

To view, visit change 498855. To unsubscribe, or for help writing mail filters, visit settings.

Zvonimir Pavlinovic (Gerrit)

unread,

May 30, 2023, 1:43:25 PM5/30/23

to Jonathan Amsterdam, goph...@pubsubhelper.golang.org, Gopher Robot, Maceo Thompson, golang-co...@googlegroups.com

Attention is currently required from: Jonathan Amsterdam, Maceo Thompson.

Patch set 2:Code-Review +2

View Change

To view, visit change 498855. To unsubscribe, or for help writing mail filters, visit settings.

Jonathan Amsterdam (Gerrit)

unread,

May 30, 2023, 2:36:38 PM5/30/23

to goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Gopher Robot, Zvonimir Pavlinovic, Maceo Thompson, golang-co...@googlegroups.com

Jonathan Amsterdam submitted this change.

View Change

Approvals:
  Jonathan Amsterdam: Run TryBots
  Zvonimir Pavlinovic: Looks good to me, approved
  Gopher Robot: TryBots succeeded

cmd/jobs: simplify upload logic

Change-Id: Ia217b376a5de5d7559db74746ae14545eea6242b

Reviewed-on: https://go-review.googlesource.com/c/pkgsite-metrics/+/498855
Run-TryBot: Jonathan Amsterdam <j...@google.com>
TryBot-Result: Gopher Robot <go...@golang.org>
Reviewed-by: Zvonimir Pavlinovic <zpavl...@google.com>


---
M cmd/jobs/main.go
1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/cmd/jobs/main.go b/cmd/jobs/main.go
index e34bafb..cdc99a7 100644
--- a/cmd/jobs/main.go
+++ b/cmd/jobs/main.go
@@ -207,7 +207,6 @@


 // As an optimization, it skips the upload if the file is already on GCS
 // and has the same checksum as the local file.
 func uploadAnalysisBinary(ctx context.Context, binaryFile string) error {
-	var upload bool
 	const bucketName = projectID
 	binaryName := filepath.Base(binaryFile)
 	objectName := path.Join("analysis-binaries", binaryName)

@@ -226,7 +225,6 @@


 	attrs, err := object.Attrs(ctx)
 	if errors.Is(err, storage.ErrObjectNotExist) {
 		fmt.Printf("%s does not exist, uploading\n", object.ObjectName())
-		upload = true
 	} else if err != nil {
 		return err
 	} else if g, w := len(attrs.MD5), md5.Size; g != w {

@@ -236,20 +234,15 @@


 		if err != nil {
 			return err
 		}
-		upload = !bytes.Equal(localMD5, attrs.MD5)
-		if upload {
-			fmt.Printf("binary %s exists on GCS but hashes don't match; uploading\n", binaryName)
-		} else {
+		if bytes.Equal(localMD5, attrs.MD5) {
 			fmt.Printf("%s already on GCS with same checksum; not uploading\n", binaryFile)
+			return nil
+		} else {
+			fmt.Printf("binary %s exists on GCS but hashes don't match; uploading\n", binaryName)
 		}
 	}
-	if upload {
-		if err := copyToGCS(ctx, object, binaryFile); err != nil {
-			return err
-		}
-		fmt.Printf("copied %s to %s\n", binaryFile, object.ObjectName())
-	}
-	return nil
+	fmt.Printf("copying %s to %s\n", binaryFile, object.ObjectName())
+	return copyToGCS(ctx, object, binaryFile)
 }
 
 // fileMD5 computes the MD5 checksum of the given file.

To view, visit change 498855. To unsubscribe, or for help writing mail filters, visit settings.

Reply all

Reply to author

Forward