You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dolphinscheduler.apache.org by GitBox <gi...@apache.org> on 2022/09/28 09:46:53 UTC

[GitHub] [dolphinscheduler] EricGao888 opened a new issue, #12190: [Improvement][SPI] Remove duplicate PropertyUtils

EricGao888 opened a new issue, #12190:
URL: https://github.com/apache/dolphinscheduler/issues/12190

   ### Search before asking
   
   - [X] I had searched in the [issues](https://github.com/apache/dolphinscheduler/issues?q=is%3Aissue) and found no similar feature requirement.
   
   
   ### Description
   
   * Currently we have two `PropertyUtils` in `common` and `spi` module.
   * It is hard to maintain duplicate util classes. 
   * Furthermore, `PropertyUtils` in `spi` module gives some kind of initialization error when we try to mock it using `Mockito.mockStatic()`. Instead of spending time finding tricky ways to  mock it, I suggest we remove `PropertyUtils` in `spi` module.
   
   ### Are you willing to submit a PR?
   
   - [X] Yes I am willing to submit a PR!
   
   ### Code of Conduct
   
   - [X] I agree to follow this project's [Code of Conduct](https://www.apache.org/foundation/policies/conduct)
   


-- 
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: commits-unsubscribe@dolphinscheduler.apache.org.apache.org

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


[GitHub] [dolphinscheduler] EricGao888 commented on issue #12190: [Improvement][SPI] Remove duplicate PropertyUtils

Posted by GitBox <gi...@apache.org>.
EricGao888 commented on issue #12190:
URL: https://github.com/apache/dolphinscheduler/issues/12190#issuecomment-1272434504

   > How about make `task-api` rely on the common module?
   
   FYI, here are the files of `task-api` which `common` depends on:
   
   ```text
   dolphinscheduler.plugin.task.api.enums.TaskExecutionStatus;
   dolphinscheduler.plugin.task.api.enums.TaskTimeoutStrategy;
   dolphinscheduler.plugin.task.api.parameters.TaskTimeoutParameter;
   dolphinscheduler.plugin.task.api.enums.DataType;
   dolphinscheduler.plugin.task.api.enums.Direct;
   dolphinscheduler.plugin.task.api.model.Property;
   dolphinscheduler.plugin.task.api.TaskConstants;
   dolphinscheduler.plugin.task.api.parser.PlaceholderUtils;
   dolphinscheduler.plugin.task.api.parser.TimePlaceholderUtils;
   ```
   
   I need more time to look into them to see whether to move those constant and utility stuff into `common` module or move the code depend on them into `task-api` module.


-- 
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: commits-unsubscribe@dolphinscheduler.apache.org

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


[GitHub] [dolphinscheduler] EricGao888 commented on issue #12190: [Improvement][SPI] Remove duplicate PropertyUtils

Posted by GitBox <gi...@apache.org>.
EricGao888 commented on issue #12190:
URL: https://github.com/apache/dolphinscheduler/issues/12190#issuecomment-1274715215

   > I'm working on this to move classes in common module to more suitable modules, and remove the duplicated PropertyUtils
   
   @kezhenxu94 Cool! Thanks. 


-- 
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: commits-unsubscribe@dolphinscheduler.apache.org

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


[GitHub] [dolphinscheduler] EricGao888 commented on issue #12190: [Improvement][SPI] Remove duplicate PropertyUtils

Posted by GitBox <gi...@apache.org>.
EricGao888 commented on issue #12190:
URL: https://github.com/apache/dolphinscheduler/issues/12190#issuecomment-1260867024

   Looks like we will have a cyclic reference if adding `common` dependency into `task-plugin` module:
   
   ```
   [ERROR] [ERROR] The projects in the reactor contain a cyclic reference: Edge between 'Vertex{label='org.apache.dolphinscheduler:dolphinscheduler-common:dev-SNAPSHOT'}' and 'Vertex{label='org.apache.dolphinscheduler:dolphinscheduler-task-api:dev-SNAPSHOT'}' introduces to cycle in the graph org.apache.dolphinscheduler:dolphinscheduler-task-api:dev-SNAPSHOT --> org.apache.dolphinscheduler:dolphinscheduler-common:dev-SNAPSHOT --> org.apache.dolphinscheduler:dolphinscheduler-task-api:dev-SNAPSHOT @ 
   [ERROR] The projects in the reactor contain a cyclic reference: Edge between 'Vertex{label='org.apache.dolphinscheduler:dolphinscheduler-common:dev-SNAPSHOT'}' and 'Vertex{label='org.apache.dolphinscheduler:dolphinscheduler-task-api:dev-SNAPSHOT'}' introduces to cycle in the graph org.apache.dolphinscheduler:dolphinscheduler-task-api:dev-SNAPSHOT --> org.apache.dolphinscheduler:dolphinscheduler-common:dev-SNAPSHOT --> org.apache.dolphinscheduler:dolphinscheduler-task-api:dev-SNAPSHOT -> [Help 1]
   
   ```
   
   ![image](https://user-images.githubusercontent.com/34905992/192783331-d0c9dbdf-40c3-4547-a6c3-473ce65262ff.png)
   
   @kezhenxu94 @ruanwenjun @caishunfeng @SbloodyS May I ask whether there are any grace solutions to this? Thanks 


-- 
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: commits-unsubscribe@dolphinscheduler.apache.org

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


[GitHub] [dolphinscheduler] kezhenxu94 commented on issue #12190: [Improvement][SPI] Remove duplicate PropertyUtils

Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on issue #12190:
URL: https://github.com/apache/dolphinscheduler/issues/12190#issuecomment-1274709139

   I'm working on this to move classes in common module to more suitable modules, and remove the duplicated PropertyUtils


-- 
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: commits-unsubscribe@dolphinscheduler.apache.org

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


[GitHub] [dolphinscheduler] github-actions[bot] commented on issue #12190: [Improvement][SPI] Remove duplicate PropertyUtils

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on issue #12190:
URL: https://github.com/apache/dolphinscheduler/issues/12190#issuecomment-1260811699

   Thank you for your feedback, we have received your issue, Please wait patiently for a reply.
   * In order for us to understand your request as soon as possible, please provide detailed information、version or pictures.
   * If you haven't received a reply for a long time, you can [join our slack](https://s.apache.org/dolphinscheduler-slack) and send your question to channel `#troubleshooting`


-- 
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: commits-unsubscribe@dolphinscheduler.apache.org

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


[GitHub] [dolphinscheduler] ruanwenjun commented on issue #12190: [Improvement][SPI] Remove duplicate PropertyUtils

Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on issue #12190:
URL: https://github.com/apache/dolphinscheduler/issues/12190#issuecomment-1272430967

   How about make `task-api` relying on the common module? 


-- 
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: commits-unsubscribe@dolphinscheduler.apache.org

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


[GitHub] [dolphinscheduler] kezhenxu94 commented on issue #12190: [Improvement][SPI] Remove duplicate PropertyUtils

Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on issue #12190:
URL: https://github.com/apache/dolphinscheduler/issues/12190#issuecomment-1263374813

   > Looks like we will have a cyclic reference if adding `common` dependency into `task-plugin` module:
   > 
   > ```
   > [ERROR] [ERROR] The projects in the reactor contain a cyclic reference: Edge between 'Vertex{label='org.apache.dolphinscheduler:dolphinscheduler-common:dev-SNAPSHOT'}' and 'Vertex{label='org.apache.dolphinscheduler:dolphinscheduler-task-api:dev-SNAPSHOT'}' introduces to cycle in the graph org.apache.dolphinscheduler:dolphinscheduler-task-api:dev-SNAPSHOT --> org.apache.dolphinscheduler:dolphinscheduler-common:dev-SNAPSHOT --> org.apache.dolphinscheduler:dolphinscheduler-task-api:dev-SNAPSHOT @ 
   > [ERROR] The projects in the reactor contain a cyclic reference: Edge between 'Vertex{label='org.apache.dolphinscheduler:dolphinscheduler-common:dev-SNAPSHOT'}' and 'Vertex{label='org.apache.dolphinscheduler:dolphinscheduler-task-api:dev-SNAPSHOT'}' introduces to cycle in the graph org.apache.dolphinscheduler:dolphinscheduler-task-api:dev-SNAPSHOT --> org.apache.dolphinscheduler:dolphinscheduler-common:dev-SNAPSHOT --> org.apache.dolphinscheduler:dolphinscheduler-task-api:dev-SNAPSHOT -> [Help 1]
   > ```
   > 
   > ![image](https://user-images.githubusercontent.com/34905992/192783331-d0c9dbdf-40c3-4547-a6c3-473ce65262ff.png)
   > 
   > @kezhenxu94 @ruanwenjun @caishunfeng @SbloodyS May I ask whether there are any graceful solutions to this? Thanks
   
   No, it's a really bad idea to work around this even if there is a solution. There is no reason the common module should depend on the task module, anything inside common module that uses task module should be moved to task-api module.


-- 
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: commits-unsubscribe@dolphinscheduler.apache.org

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


[GitHub] [dolphinscheduler] ruanwenjun closed issue #12190: [Improvement][SPI] Remove duplicate PropertyUtils

Posted by GitBox <gi...@apache.org>.
ruanwenjun closed issue #12190: [Improvement][SPI] Remove duplicate PropertyUtils
URL: https://github.com/apache/dolphinscheduler/issues/12190


-- 
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: commits-unsubscribe@dolphinscheduler.apache.org

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