[GitHub] [beam] ibzib commented on a change in pull request #15223: [BEAM-12662] Get Flink version from cluster.

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

ibzib commented on a change in pull request #15223:
URL: https://github.com/apache/beam/pull/15223#discussion_r697753816



##########
File path: sdks/python/apache_beam/runners/portability/flink_uber_jar_job_server.py
##########
@@ -68,7 +68,7 @@ def executable_jar(self):
               'the scheme is specified. If using a local file path, make sure '
               'the file exists; you may have to first build the job server '
               'using `./gradlew runners:flink:%s:job-server:shadowJar`.' %
-              (self._executable_jar, self._flink_version))
+              (self._executable_jar, self.flink_version())

Review comment:
       Since `flink_version()` sends a request to the Flink rest endpoint, I don't think we can ever guarantee the call is going to work. I'm thinking we should do both:
   
   1. We can try to give an error message with the correct Flink version, but if `flink_version()` fails, give a message with a generic `1.x`.
   2. Provide an actionable error message when the rest calls fail ("start a Flink cluster with rest enabled" etc.). This doesn't have to happen in this PR though, so I filed a separate ticket https://issues.apache.org/jira/browse/BEAM-12813.




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