[GitHub] [beam] lostluck commented on a diff in pull request #26062: [#26061][Go SDK] Return server error on stream close for artifact upload.

["lostluck (via GitHub)" <gi...@apache.org>]

lostluck commented on code in PR #26062:
URL: https://github.com/apache/beam/pull/26062#discussion_r1157382334


##########
sdks/go/pkg/beam/runners/universal/runnerlib/stage.go:
##########
@@ -107,10 +139,10 @@ func StageViaPortableApi(ctx context.Context, cc *grpc.ClientConn, binary, st st
 	}
 }
 
-func StageFile(filename string, stream jobpb.ArtifactStagingService_ReverseArtifactRetrievalServiceClient) error {
+func stageFile(filename string, stream jobpb.ArtifactStagingService_ReverseArtifactRetrievalServiceClient) error {

Review Comment:
   Not in the least.
   
   1. It's deep within the runner lib.
   2. We're the only ones that depend on it in open source land.
   3. It's using the fairly esoteric (WRT beam knowledge) Reverse Artifact Retreival service, which while it works, isn't something many have experience with.
   4. It was brittle to begin with (erroring in ways that hid server connection closes).
   5. It shouldn't have been exposed in the first place.
   6. It was also undocumented, which doesn't encourage use...



-- 
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