zhuangchong commented on pull request #3243:
URL: 
https://github.com/apache/incubator-dolphinscheduler/pull/3243#issuecomment-662258262


   @yangyichao-mango
   
   1. I haven't used the method of creating API directly. It's not clear. I see 
that the method of checkprocessnodelist in dolphinscheduler-api  module can 
also detect parameters. I don't know if this method will be used to create API 
directly. You can have a look.   
   
   ```
   public static boolean checkTaskNodeParameters(String parameter, String 
taskType) {
       AbstractParameters abstractParameters = 
TaskParametersUtils.getParameters(taskType, parameter);
   
       if (abstractParameters != null) {
         return abstractParameters.checkParameters();
       }
   
       return false;
     }
   ```
   
   2. This PR is to add an exception log in sqltask
   ```
        try {
                   resultSet.close();                  
               } catch (SQLException e) {                  
   
    +              logger.error("close result set error :",e);
               }
   ```
   I think more should be added resultSet==null,pstmt==null, connection==null
   ```
        try {
                   resultSet.close();                  
               } catch (SQLException e) {                  
    +            resultSet == null;
    +              logger.error("close result set error :",e);
               }         
    ``` 
   ---
   1. 你说的api直接创建这种方式我没有用过,这个不太清楚,我看dolphinscheduler-api 
模块里面有checkProcessNodeList方法也会检测参数。不知道你说的api直接创建会不会走这个方法,你可以看一下。
   ```
   public static boolean checkTaskNodeParameters(String parameter, String 
taskType) {
       AbstractParameters abstractParameters = 
TaskParametersUtils.getParameters(taskType, parameter);
   
       if (abstractParameters != null) {
         return abstractParameters.checkParameters();
       }
   
       return false;
     }
   ```
   2. 本次PR是在SqlTask中增加 异常日志
   ```
        try {
                   resultSet.close();                  
               } catch (SQLException e) {                  
   
    +              logger.error("close result set error :",e);
               }
   ```
   我认为更应该增加的是resultSet==null,pstmt==null, connection==null
   ```
        try {
                   resultSet.close();                  
               } catch (SQLException e) {                  
    +            resultSet == null;
    +              logger.error("close result set error :",e);
               }         
    ``` 
   
   
   


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

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to