[GitHub] [beam] lostluck commented on a change in pull request #15323: [BEAM-5379] Go SDK modules support.

[GitBox <gi...@apache.org>]

lostluck commented on a change in pull request #15323:
URL: https://github.com/apache/beam/pull/15323#discussion_r687822810



##########
File path: sdks/go.mod
##########
@@ -0,0 +1,31 @@
+module github.com/apache/beam/sdks

Review comment:
       RAT's probably complaining about no LICENSE on this file. Since we can have comments in go.mod files, we can just add a license header as normal.
   
   If it still complains, add it to the filter:
   
   https://github.com/apache/beam/blob/10e60255683ed14358537ce17ecda54460bebf40/build.gradle.kts#L63
   
   Probably won't though as the katas have a mod:
   https://github.com/apache/beam/blob/10e60255683ed14358537ce17ecda54460bebf40/learning/katas/go/go.mod

##########
File path: sdks/go.mod
##########
@@ -0,0 +1,31 @@
+module github.com/apache/beam/sdks
+
+go 1.16
+
+require (
+	cloud.google.com/go v0.88.0 // indirect
+	cloud.google.com/go/bigquery v1.19.0
+	cloud.google.com/go/datastore v1.5.0
+	cloud.google.com/go/pubsub v1.13.0
+	cloud.google.com/go/storage v1.16.0
+	github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
+	github.com/golang/protobuf v1.5.2
+	github.com/golang/snappy v0.0.4 // indirect
+	github.com/google/go-cmp v0.5.6
+	github.com/google/uuid v1.3.0
+	github.com/kr/text v0.2.0 // indirect
+	github.com/linkedin/goavro v2.1.0+incompatible
+	github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e // indirect
+	github.com/nightlyone/lockfile v1.0.0 // indirect
+	github.com/spf13/cobra v1.2.1
+	golang.org/x/net v0.0.0-20210726213435-c6fcb2dbf985

Review comment:
       Likely need to set an older version of this module if I understand the complaint the PostCommit build has correctly.
   
   Note how it's tagged at a pseudo version of... July 26th.

##########
File path: sdks/go/pkg/beam/model/PROTOBUF.md
##########
@@ -24,44 +24,38 @@ If you make changes to .proto files, you will need to rebuild the generated Go c
 First, follow this one-time setup:
 
 1. Download [the protobuf compiler](https://github.com/google/protobuf/releases).
-   The simplest approach is to download one of the prebuilt binaries and extract
-   it somewhere in your machine's `$PATH`.
-1. A properly installed Go development environment per [the official
-   instructions](https://golang.org/doc/install). `$GOPATH` must be set properly.
-   If it's not set, follow
-   [these instructions](https://github.com/golang/go/wiki/SettingGOPATH).
+   The simplest approach is to download one of the prebuilt binaries (named
+   `protoc`) and extract it somewhere in your machine's `$PATH`.
 1. Add `$GOBIN` to your `$PATH`. (Note: If `$GOBIN` is not set, add `$GOPATH/bin`
    instead.)
 
 To generate the code:
 
 1. Navigate to this directory (`pkg/beam/model`).
-1. `go get -u github.com/golang/protobuf/protoc-gen-go`
+1. `go install google.golang.org/protobuf/cmd/protoc-gen-go@latest`
+1. `go install google.golang.org/grpc/cmd/protoc-gen-go-grpc@latest`

Review comment:
       Instead of recommending @latest, we should have the instructions ensure the go.mod version is being used which is the default behavior as of go 1.16:
   
   https://golang.org/ref/mod#go-install
   
   IIUC this means adding a required version to our go.mod in the chance we aren't already depending on a specific version.
   
   That would avoid version random increments, and we can manage the updates more intentionally, and hopefully, get folks to do this more automatically when they change the protos. 
   
   Especially since the instructions below for failures end up saying "either move to the go mod version, or update the go mod version".

##########
File path: sdks/java/container/build.gradle
##########
@@ -55,7 +54,7 @@ dependencies {
 }
 
 golang {
-  packagePath = 'github.com/apache/beam/sdks/java/boot'
+  packagePath = 'github.com/apache/beam/sdks/java/container'

Review comment:
       It's astounding things were still working with that directory being wrong?

##########
File path: sdks/go.mod
##########
@@ -0,0 +1,31 @@
+module github.com/apache/beam/sdks

Review comment:
       It would also be good to add a comment about the module placement to this file after the license as well (covers SDK boot code, vendor issue)




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org