[GitHub] [incubator-streampark] zhoulii commented on a diff in pull request #2705: [improve-2695][console-service] The flink/pipe/build interface is not standardized

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

zhoulii commented on code in PR #2705:
URL: https://github.com/apache/incubator-streampark/pull/2705#discussion_r1188063430


##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/core/service/impl/AppBuildPipeServiceImpl.java:
##########
@@ -130,6 +131,8 @@
 
   @Autowired private ResourceService resourceService;
 
+  @Autowired private AppBuildPipeService appBuildPipeService;

Review Comment:
   the injection of `AppBuildPipeService` is unnecessary.



##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/core/service/impl/AppBuildPipeServiceImpl.java:
##########
@@ -342,6 +376,38 @@ public void onDockerPushProgressChange(DockerPushSnapshot snapshot) {
     return saved;
   }
 
+  /**
+   * check the build environment
+   *
+   * @param appId application id
+   * @param forceBuild forced start pipeline or not
+   * @return
+   */
+  @Override
+  public void checkBuildEnv(Long appId, boolean forceBuild) {
+    Application app = applicationService.getById(appId);
+
+    // 1) check flink version
+    FlinkEnv env = flinkEnvService.getById(app.getVersionId());
+    boolean checkVersion = env.getFlinkVersion().checkVersion(false);
+    if (!checkVersion) {
+      throw new ApiAlertException("Unsupported flink version: " + env.getFlinkVersion().version());
+    }
+
+    // 2) check env
+    boolean envOk = applicationService.checkEnv(app);
+    if (!envOk) {
+      throw new ApiAlertException(

Review Comment:
   same as above



##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/core/service/impl/AppBuildPipeServiceImpl.java:
##########
@@ -342,6 +376,38 @@ public void onDockerPushProgressChange(DockerPushSnapshot snapshot) {
     return saved;
   }
 
+  /**
+   * check the build environment
+   *
+   * @param appId application id
+   * @param forceBuild forced start pipeline or not
+   * @return
+   */
+  @Override
+  public void checkBuildEnv(Long appId, boolean forceBuild) {
+    Application app = applicationService.getById(appId);
+
+    // 1) check flink version
+    FlinkEnv env = flinkEnvService.getById(app.getVersionId());
+    boolean checkVersion = env.getFlinkVersion().checkVersion(false);
+    if (!checkVersion) {
+      throw new ApiAlertException("Unsupported flink version: " + env.getFlinkVersion().version());
+    }

Review Comment:
   we can use `ApiAlertException.throwIfFalse` to make code more concise.



##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/core/service/AppBuildPipeService.java:
##########
@@ -33,9 +31,23 @@
 
 public interface AppBuildPipeService extends IService<AppBuildPipeline> {
 
-  /** Build application. This is an async call method. */
-  boolean buildApplication(@Nonnull Application app, ApplicationLog applicationLog)
-      throws Exception;
+  /**
+   * Build application. This is an async call method.
+   *
+   * @param appId application id
+   * @param forceBuild forced start pipeline or not
+   * @return Whether the pipeline was successfully started
+   */
+  boolean buildApplication(@Nonnull Long appId, boolean forceBuild) throws Exception;
+
+  /**
+   * check the build environment
+   *
+   * @param appId application id
+   * @param forceBuild forced start pipeline or not
+   * @return
+   */
+  void checkBuildEnv(Long appId, boolean forceBuild);

Review Comment:
   since this method is only used by `buildApplication` and no controller invokes it, we can make it a private method in AppBuildPipeServiceImpl.java



-- 
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: issues-unsubscribe@streampark.apache.org

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