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


##########
sdks/go/pkg/beam/runners/prism/internal/worker/worker.go:
##########
@@ -727,6 +732,10 @@ func NewMultiplexW(lis net.Listener, g *grpc.Server, 
logger *slog.Logger) *Multi
 func (mw *MultiplexW) MakeWorker(id, env string) *W {
        mw.mu.Lock()
        defer mw.mu.Unlock()
+       jobId := strings.TrimSuffix(id, "_"+env)

Review Comment:
   Ah. Yup..I misread that. GopherCon is very busy.
   
   I think this is a bit brittle/tricky. Don't worry too much about changing 
the "public" api. Prism lives in an internal package, limiting the scope where 
things are called. That means you can easily change all the callsites if we 
need to.
   
   It doesn't look like we rebuild that ID anywhere else, which means we can 
make that an internal detail to how the workers manage themselves, instead of 
something the job manages.



-- 
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-unsubscr...@beam.apache.org

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

Reply via email to