youngoli commented on a change in pull request #16228:
URL: https://github.com/apache/beam/pull/16228#discussion_r770172513
##########
File path: sdks/go/pkg/beam/core/runtime/xlangx/expansionx/download.go
##########
@@ -58,6 +58,10 @@ func GetBeamJar(gradleTarget, version string) (string,
error) {
}
func (j *jarGetter) getJar(gradleTarget, version string) (string, error) {
+ if strings.Contains(version, ".dev") {
Review comment:
While I support having a clearer error message for users, it feels a bit
limiting to just disallow any container with ".dev" outright. What about
approaching this as letting the code fail naturally, and if it fails and we
detect that the user was attempting to retrieve a .dev container, then we add
this message.
You'd need to run this code with a .dev container to see where exactly it
fails (probably on http.Get if I had to guess). But then all you have to do is
move this if statement to that error block, and if it's true then wrapping the
error with a
[SetTopLevelMsg](https://github.com/apache/beam/blob/master/sdks/go/pkg/beam/internal/errors/errors.go#L90)
function should do it. (I originally wrote that function for this exact
purpose; adding messages directly to users to tell them how to fix a specific
error).
That changes this behavior from "disallowing something" to "detecting a
common error case".
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
