You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by GitBox <gi...@apache.org> on 2020/10/18 06:25:50 UTC

[GitHub] [shardingsphere-elasticjob] wwj-go opened a new pull request #1567: Refactor ElasticJobServiceLoader, remove unused code and use Optional…

wwj-go opened a new pull request #1567:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1567


   … instead of throw JobConfigurationException (#1566)
   
   Fixes #1566. 
   Refactor `org.apache.shardingsphere.elasticjob.infra.spi.ElasticJobServiceLoader`
   Changes proposed in this pull request:
   - Remove unused code.
   - Refactor it method to return an empty Optional instance instead of throw JobConfigurationException when cannot find the service instance.
   


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [shardingsphere-elasticjob] wwj-go commented on a change in pull request #1567: Refactor ElasticJobServiceLoader, remove unused code and use Optional…

Posted by GitBox <gi...@apache.org>.
wwj-go commented on a change in pull request #1567:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1567#discussion_r507029272



##########
File path: elasticjob-infra/elasticjob-infra-common/src/main/java/org/apache/shardingsphere/elasticjob/infra/spi/ElasticJobServiceLoader.java
##########
@@ -106,33 +61,42 @@ private static Object newServiceInstance(final Class<?> clazz) {
     /**
      * Get cached instance.
      *
-     * @param typedService service type
+     * @param typedServiceInterface typed service interface
      * @param type         specific service type
      * @param <T>          specific type of service
      * @return cached service instance
      */
-    public static <T extends TypedSPI> T getCachedInstance(final Class<T> typedService, final String type) {
-        T instance = TYPED_SERVICES.containsKey(typedService) ? (T) TYPED_SERVICES.get(typedService).get(type) : null;
+    public static <T extends TypedSPI> Optional<T> getCachedTypedServiceInstance(final Class<T> typedServiceInterface, final String type) {
+        T instance = TYPED_SERVICES.containsKey(typedServiceInterface) ? (T) TYPED_SERVICES.get(typedServiceInterface).get(type) : null;

Review comment:
       That looks good.




----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [shardingsphere-elasticjob] codecov-io edited a comment on pull request #1567: Refactor ElasticJobServiceLoader, remove unused code and use Optional…

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #1567:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1567#issuecomment-711124309


   # [Codecov](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567?src=pr&el=h1) Report
   > Merging [#1567](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567?src=pr&el=desc) into [master](https://codecov.io/gh/apache/shardingsphere-elasticjob/commit/8fdd64868b0dea21d9d4770968412d49b8fea33b?el=desc) will **increase** coverage by `0.18%`.
   > The diff coverage is `93.15%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567/graphs/tree.svg?width=650&height=150&src=pr&token=8ZMVc4Yo4Z)](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #1567      +/-   ##
   ============================================
   + Coverage     85.39%   85.57%   +0.18%     
     Complexity      103      103              
   ============================================
     Files           250      250              
     Lines          5688     5676      -12     
     Branches        882      875       -7     
   ============================================
     Hits           4857     4857              
   + Misses          515      507       -8     
   + Partials        316      312       -4     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...hardingsphere/elasticjob/api/JobConfiguration.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2VsYXN0aWNqb2IvYXBpL0pvYkNvbmZpZ3VyYXRpb24uamF2YQ==) | `83.72% <ø> (ø)` | `0.00 <0.00> (ø)` | |
   | [.../error/handler/dingtalk/DingtalkConfiguration.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1lcnJvci1oYW5kbGVyL2VsYXN0aWNqb2ItZXJyb3ItaGFuZGxlci10eXBlL2VsYXN0aWNqb2ItZXJyb3ItaGFuZGxlci1kaW5ndGFsay9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZWxhc3RpY2pvYi9lcnJvci9oYW5kbGVyL2Rpbmd0YWxrL0Rpbmd0YWxrQ29uZmlndXJhdGlvbi5qYXZh) | `100.00% <ø> (ø)` | `0.00 <0.00> (ø)` | |
   | [...cjob/error/handler/dingtalk/DingtalkConstants.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1lcnJvci1oYW5kbGVyL2VsYXN0aWNqb2ItZXJyb3ItaGFuZGxlci10eXBlL2VsYXN0aWNqb2ItZXJyb3ItaGFuZGxlci1kaW5ndGFsay9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZWxhc3RpY2pvYi9lcnJvci9oYW5kbGVyL2Rpbmd0YWxrL0Rpbmd0YWxrQ29uc3RhbnRzLmphdmE=) | `66.66% <ø> (ø)` | `0.00 <0.00> (ø)` | |
   | [...elasticjob/error/handler/email/EmailConstants.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1lcnJvci1oYW5kbGVyL2VsYXN0aWNqb2ItZXJyb3ItaGFuZGxlci10eXBlL2VsYXN0aWNqb2ItZXJyb3ItaGFuZGxlci1lbWFpbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZWxhc3RpY2pvYi9lcnJvci9oYW5kbGVyL2VtYWlsL0VtYWlsQ29uc3RhbnRzLmphdmE=) | `0.00% <ø> (ø)` | `0.00 <0.00> (ø)` | |
   | [...b/error/handler/general/IgnoreJobErrorHandler.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1lcnJvci1oYW5kbGVyL2VsYXN0aWNqb2ItZXJyb3ItaGFuZGxlci10eXBlL2VsYXN0aWNqb2ItZXJyb3ItaGFuZGxlci1nZW5lcmFsL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9lbGFzdGljam9iL2Vycm9yL2hhbmRsZXIvZ2VuZXJhbC9JZ25vcmVKb2JFcnJvckhhbmRsZXIuamF2YQ==) | `100.00% <ø> (ø)` | `1.00 <0.00> (ø)` | |
   | [...ob/error/handler/general/ThrowJobErrorHandler.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1lcnJvci1oYW5kbGVyL2VsYXN0aWNqb2ItZXJyb3ItaGFuZGxlci10eXBlL2VsYXN0aWNqb2ItZXJyb3ItaGFuZGxlci1nZW5lcmFsL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9lbGFzdGljam9iL2Vycm9yL2hhbmRsZXIvZ2VuZXJhbC9UaHJvd0pvYkVycm9ySGFuZGxlci5qYXZh) | `100.00% <ø> (ø)` | `1.00 <0.00> (ø)` | |
   | [...cjob/error/handler/wechat/WechatConfiguration.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1lcnJvci1oYW5kbGVyL2VsYXN0aWNqb2ItZXJyb3ItaGFuZGxlci10eXBlL2VsYXN0aWNqb2ItZXJyb3ItaGFuZGxlci13ZWNoYXQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2VsYXN0aWNqb2IvZXJyb3IvaGFuZGxlci93ZWNoYXQvV2VjaGF0Q29uZmlndXJhdGlvbi5qYXZh) | `100.00% <ø> (ø)` | `0.00 <0.00> (ø)` | |
   | [...asticjob/error/handler/wechat/WechatConstants.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1lcnJvci1oYW5kbGVyL2VsYXN0aWNqb2ItZXJyb3ItaGFuZGxlci10eXBlL2VsYXN0aWNqb2ItZXJyb3ItaGFuZGxlci13ZWNoYXQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2VsYXN0aWNqb2IvZXJyb3IvaGFuZGxlci93ZWNoYXQvV2VjaGF0Q29uc3RhbnRzLmphdmE=) | `66.66% <ø> (ø)` | `0.00 <0.00> (ø)` | |
   | [...sphere/elasticjob/infra/concurrent/BlockUtils.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1pbmZyYS9lbGFzdGljam9iLWluZnJhLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZWxhc3RpY2pvYi9pbmZyYS9jb25jdXJyZW50L0Jsb2NrVXRpbHMuamF2YQ==) | `100.00% <ø> (ø)` | `0.00 <0.00> (ø)` | |
   | [...sphere/elasticjob/infra/context/ExecutionType.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1pbmZyYS9lbGFzdGljam9iLWluZnJhLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZWxhc3RpY2pvYi9pbmZyYS9jb250ZXh0L0V4ZWN1dGlvblR5cGUuamF2YQ==) | `100.00% <ø> (ø)` | `0.00 <0.00> (ø)` | |
   | ... and [32 more](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567?src=pr&el=footer). Last update [8fdd648...4276242](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [shardingsphere-elasticjob] wwj-go closed pull request #1567: Refactor ElasticJobServiceLoader, remove unused code and use Optional…

Posted by GitBox <gi...@apache.org>.
wwj-go closed pull request #1567:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1567


   


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [shardingsphere-elasticjob] TeslaCN commented on a change in pull request #1567: Refactor ElasticJobServiceLoader, remove unused code and use Optional…

Posted by GitBox <gi...@apache.org>.
TeslaCN commented on a change in pull request #1567:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1567#discussion_r507024721



##########
File path: elasticjob-infra/elasticjob-infra-common/src/main/java/org/apache/shardingsphere/elasticjob/infra/handler/threadpool/JobExecutorServiceHandlerFactory.java
##########
@@ -42,8 +43,11 @@
      */
     public static JobExecutorServiceHandler getHandler(final String type) {
         if (Strings.isNullOrEmpty(type)) {
-            return ElasticJobServiceLoader.getCachedInstance(JobExecutorServiceHandler.class, DEFAULT_HANDLER);
+            return ElasticJobServiceLoader.getCachedTypedServiceInstance(JobExecutorServiceHandler.class, DEFAULT_HANDLER)

Review comment:
       Is it necessary to check if default handler present? Why could this happen?

##########
File path: elasticjob-infra/elasticjob-infra-common/src/main/java/org/apache/shardingsphere/elasticjob/infra/spi/ElasticJobServiceLoader.java
##########
@@ -106,33 +61,42 @@ private static Object newServiceInstance(final Class<?> clazz) {
     /**
      * Get cached instance.
      *
-     * @param typedService service type
+     * @param typedServiceInterface typed service interface
      * @param type         specific service type
      * @param <T>          specific type of service
      * @return cached service instance
      */
-    public static <T extends TypedSPI> T getCachedInstance(final Class<T> typedService, final String type) {
-        T instance = TYPED_SERVICES.containsKey(typedService) ? (T) TYPED_SERVICES.get(typedService).get(type) : null;
+    public static <T extends TypedSPI> Optional<T> getCachedTypedServiceInstance(final Class<T> typedServiceInterface, final String type) {
+        T instance = TYPED_SERVICES.containsKey(typedServiceInterface) ? (T) TYPED_SERVICES.get(typedServiceInterface).get(type) : null;

Review comment:
       Consider making good use of `Optional#map` if you are going to use `Optional`.

##########
File path: elasticjob-infra/elasticjob-infra-common/src/main/java/org/apache/shardingsphere/elasticjob/infra/spi/ElasticJobServiceLoader.java
##########
@@ -106,33 +61,42 @@ private static Object newServiceInstance(final Class<?> clazz) {
     /**
      * Get cached instance.
      *
-     * @param typedService service type
+     * @param typedServiceInterface typed service interface
      * @param type         specific service type
      * @param <T>          specific type of service
      * @return cached service instance
      */
-    public static <T extends TypedSPI> T getCachedInstance(final Class<T> typedService, final String type) {
-        T instance = TYPED_SERVICES.containsKey(typedService) ? (T) TYPED_SERVICES.get(typedService).get(type) : null;
+    public static <T extends TypedSPI> Optional<T> getCachedTypedServiceInstance(final Class<T> typedServiceInterface, final String type) {
+        T instance = TYPED_SERVICES.containsKey(typedServiceInterface) ? (T) TYPED_SERVICES.get(typedServiceInterface).get(type) : null;
         if (null == instance) {
-            throw new JobConfigurationException("Cannot find a cached typed service instance by the interface: @" + typedService.getName() + "and type: " + type);
+            return Optional.empty();
         }
-        return instance;
+        return Optional.of(instance);
     }
 
     /**
      * New typed instance.
      *
-     * @param typedService service type
+     * @param typedServiceInterface typed service interface
      * @param type         specific service type
      * @param <T>          specific type of service
      * @return specific typed service instance
      */
-    public static <T extends TypedSPI> T newTypedServiceInstance(final Class<T> typedService, final String type) {
-        Class<?> instanceClass = TYPED_SERVICE_CLASSES.containsKey(typedService) ? TYPED_SERVICE_CLASSES.get(typedService).get(type) : null;
+    public static <T extends TypedSPI> Optional<T> newTypedServiceInstance(final Class<T> typedServiceInterface, final String type) {
+        Class<?> instanceClass = TYPED_SERVICE_CLASSES.containsKey(typedServiceInterface) ? TYPED_SERVICE_CLASSES.get(typedServiceInterface).get(type) : null;

Review comment:
       Consider making good use of `Optional#map` if you are going to use `Optional`.




----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [shardingsphere-elasticjob] codecov-io edited a comment on pull request #1567: Refactor ElasticJobServiceLoader, remove unused code and use Optional…

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #1567:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1567#issuecomment-711124309


   # [Codecov](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567?src=pr&el=h1) Report
   > Merging [#1567](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567?src=pr&el=desc) into [master](https://codecov.io/gh/apache/shardingsphere-elasticjob/commit/8fdd64868b0dea21d9d4770968412d49b8fea33b?el=desc) will **increase** coverage by `0.00%`.
   > The diff coverage is `93.15%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567/graphs/tree.svg?width=650&height=150&src=pr&token=8ZMVc4Yo4Z)](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff            @@
   ##             master    #1567   +/-   ##
   =========================================
     Coverage     85.39%   85.39%           
     Complexity      103      103           
   =========================================
     Files           250      250           
     Lines          5688     5676   -12     
     Branches        882      875    -7     
   =========================================
   - Hits           4857     4847   -10     
   - Misses          515      516    +1     
   + Partials        316      313    -3     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...hardingsphere/elasticjob/api/JobConfiguration.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2VsYXN0aWNqb2IvYXBpL0pvYkNvbmZpZ3VyYXRpb24uamF2YQ==) | `83.72% <ø> (ø)` | `0.00 <0.00> (ø)` | |
   | [.../error/handler/dingtalk/DingtalkConfiguration.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1lcnJvci1oYW5kbGVyL2VsYXN0aWNqb2ItZXJyb3ItaGFuZGxlci10eXBlL2VsYXN0aWNqb2ItZXJyb3ItaGFuZGxlci1kaW5ndGFsay9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZWxhc3RpY2pvYi9lcnJvci9oYW5kbGVyL2Rpbmd0YWxrL0Rpbmd0YWxrQ29uZmlndXJhdGlvbi5qYXZh) | `100.00% <ø> (ø)` | `0.00 <0.00> (ø)` | |
   | [...cjob/error/handler/dingtalk/DingtalkConstants.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1lcnJvci1oYW5kbGVyL2VsYXN0aWNqb2ItZXJyb3ItaGFuZGxlci10eXBlL2VsYXN0aWNqb2ItZXJyb3ItaGFuZGxlci1kaW5ndGFsay9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZWxhc3RpY2pvYi9lcnJvci9oYW5kbGVyL2Rpbmd0YWxrL0Rpbmd0YWxrQ29uc3RhbnRzLmphdmE=) | `66.66% <ø> (ø)` | `0.00 <0.00> (ø)` | |
   | [...elasticjob/error/handler/email/EmailConstants.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1lcnJvci1oYW5kbGVyL2VsYXN0aWNqb2ItZXJyb3ItaGFuZGxlci10eXBlL2VsYXN0aWNqb2ItZXJyb3ItaGFuZGxlci1lbWFpbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZWxhc3RpY2pvYi9lcnJvci9oYW5kbGVyL2VtYWlsL0VtYWlsQ29uc3RhbnRzLmphdmE=) | `0.00% <ø> (ø)` | `0.00 <0.00> (ø)` | |
   | [...b/error/handler/general/IgnoreJobErrorHandler.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1lcnJvci1oYW5kbGVyL2VsYXN0aWNqb2ItZXJyb3ItaGFuZGxlci10eXBlL2VsYXN0aWNqb2ItZXJyb3ItaGFuZGxlci1nZW5lcmFsL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9lbGFzdGljam9iL2Vycm9yL2hhbmRsZXIvZ2VuZXJhbC9JZ25vcmVKb2JFcnJvckhhbmRsZXIuamF2YQ==) | `100.00% <ø> (ø)` | `1.00 <0.00> (ø)` | |
   | [...ob/error/handler/general/ThrowJobErrorHandler.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1lcnJvci1oYW5kbGVyL2VsYXN0aWNqb2ItZXJyb3ItaGFuZGxlci10eXBlL2VsYXN0aWNqb2ItZXJyb3ItaGFuZGxlci1nZW5lcmFsL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9lbGFzdGljam9iL2Vycm9yL2hhbmRsZXIvZ2VuZXJhbC9UaHJvd0pvYkVycm9ySGFuZGxlci5qYXZh) | `100.00% <ø> (ø)` | `1.00 <0.00> (ø)` | |
   | [...cjob/error/handler/wechat/WechatConfiguration.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1lcnJvci1oYW5kbGVyL2VsYXN0aWNqb2ItZXJyb3ItaGFuZGxlci10eXBlL2VsYXN0aWNqb2ItZXJyb3ItaGFuZGxlci13ZWNoYXQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2VsYXN0aWNqb2IvZXJyb3IvaGFuZGxlci93ZWNoYXQvV2VjaGF0Q29uZmlndXJhdGlvbi5qYXZh) | `100.00% <ø> (ø)` | `0.00 <0.00> (ø)` | |
   | [...asticjob/error/handler/wechat/WechatConstants.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1lcnJvci1oYW5kbGVyL2VsYXN0aWNqb2ItZXJyb3ItaGFuZGxlci10eXBlL2VsYXN0aWNqb2ItZXJyb3ItaGFuZGxlci13ZWNoYXQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2VsYXN0aWNqb2IvZXJyb3IvaGFuZGxlci93ZWNoYXQvV2VjaGF0Q29uc3RhbnRzLmphdmE=) | `66.66% <ø> (ø)` | `0.00 <0.00> (ø)` | |
   | [...sphere/elasticjob/infra/concurrent/BlockUtils.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1pbmZyYS9lbGFzdGljam9iLWluZnJhLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZWxhc3RpY2pvYi9pbmZyYS9jb25jdXJyZW50L0Jsb2NrVXRpbHMuamF2YQ==) | `100.00% <ø> (ø)` | `0.00 <0.00> (ø)` | |
   | [...sphere/elasticjob/infra/context/ExecutionType.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1pbmZyYS9lbGFzdGljam9iLWluZnJhLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZWxhc3RpY2pvYi9pbmZyYS9jb250ZXh0L0V4ZWN1dGlvblR5cGUuamF2YQ==) | `100.00% <ø> (ø)` | `0.00 <0.00> (ø)` | |
   | ... and [34 more](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567?src=pr&el=footer). Last update [8fdd648...4276242](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [shardingsphere-elasticjob] wwj-go commented on a change in pull request #1567: Refactor ElasticJobServiceLoader, remove unused code and use Optional…

Posted by GitBox <gi...@apache.org>.
wwj-go commented on a change in pull request #1567:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1567#discussion_r507028969



##########
File path: elasticjob-infra/elasticjob-infra-common/src/main/java/org/apache/shardingsphere/elasticjob/infra/handler/threadpool/JobExecutorServiceHandlerFactory.java
##########
@@ -42,8 +43,11 @@
      */
     public static JobExecutorServiceHandler getHandler(final String type) {
         if (Strings.isNullOrEmpty(type)) {
-            return ElasticJobServiceLoader.getCachedInstance(JobExecutorServiceHandler.class, DEFAULT_HANDLER);
+            return ElasticJobServiceLoader.getCachedTypedServiceInstance(JobExecutorServiceHandler.class, DEFAULT_HANDLER)

Review comment:
       This is a redundant check and will be replaced with a simple Optional#get , as will JobShardingStrategyFactory#getStrategy




----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [shardingsphere-elasticjob] codecov-io edited a comment on pull request #1567: Refactor ElasticJobServiceLoader, remove unused code and use Optional…

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #1567:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1567#issuecomment-711124309


   # [Codecov](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567?src=pr&el=h1) Report
   > Merging [#1567](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567?src=pr&el=desc) into [master](https://codecov.io/gh/apache/shardingsphere-elasticjob/commit/8fdd64868b0dea21d9d4770968412d49b8fea33b?el=desc) will **increase** coverage by `0.11%`.
   > The diff coverage is `72.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567/graphs/tree.svg?width=650&height=150&src=pr&token=8ZMVc4Yo4Z)](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #1567      +/-   ##
   ============================================
   + Coverage     85.39%   85.50%   +0.11%     
     Complexity      103      103              
   ============================================
     Files           250      250              
     Lines          5688     5684       -4     
     Branches        882      880       -2     
   ============================================
   + Hits           4857     4860       +3     
   + Misses          515      508       -7     
     Partials        316      316              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [.../elasticjob/infra/spi/ElasticJobServiceLoader.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1pbmZyYS9lbGFzdGljam9iLWluZnJhLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZWxhc3RpY2pvYi9pbmZyYS9zcGkvRWxhc3RpY0pvYlNlcnZpY2VMb2FkZXIuamF2YQ==) | `58.33% <41.66%> (+11.45%)` | `0.00 <0.00> (ø)` | |
   | [...asticjob/error/handler/JobErrorHandlerFactory.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1lcnJvci1oYW5kbGVyL2VsYXN0aWNqb2ItZXJyb3ItaGFuZGxlci10eXBlL2VsYXN0aWNqb2ItZXJyb3ItaGFuZGxlci1nZW5lcmFsL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9lbGFzdGljam9iL2Vycm9yL2hhbmRsZXIvSm9iRXJyb3JIYW5kbGVyRmFjdG9yeS5qYXZh) | `100.00% <100.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...asticjob/executor/item/JobItemExecutorFactory.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1leGVjdXRvci9lbGFzdGljam9iLWV4ZWN1dG9yLWtlcm5lbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZWxhc3RpY2pvYi9leGVjdXRvci9pdGVtL0pvYkl0ZW1FeGVjdXRvckZhY3RvcnkuamF2YQ==) | `100.00% <100.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...a/handler/sharding/JobShardingStrategyFactory.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1pbmZyYS9lbGFzdGljam9iLWluZnJhLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZWxhc3RpY2pvYi9pbmZyYS9oYW5kbGVyL3NoYXJkaW5nL0pvYlNoYXJkaW5nU3RyYXRlZ3lGYWN0b3J5LmphdmE=) | `100.00% <100.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...r/threadpool/JobExecutorServiceHandlerFactory.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1pbmZyYS9lbGFzdGljam9iLWluZnJhLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZWxhc3RpY2pvYi9pbmZyYS9oYW5kbGVyL3RocmVhZHBvb2wvSm9iRXhlY3V0b3JTZXJ2aWNlSGFuZGxlckZhY3RvcnkuamF2YQ==) | `100.00% <100.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...cjob/infra/listener/ElasticJobListenerFactory.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1pbmZyYS9lbGFzdGljam9iLWluZnJhLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZWxhc3RpY2pvYi9pbmZyYS9saXN0ZW5lci9FbGFzdGljSm9iTGlzdGVuZXJGYWN0b3J5LmphdmE=) | `100.00% <100.00%> (ø)` | `0.00 <0.00> (ø)` | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567?src=pr&el=footer). Last update [8fdd648...eda2b19](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [shardingsphere-elasticjob] codecov-io commented on pull request #1567: Refactor ElasticJobServiceLoader, remove unused code and use Optional…

Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #1567:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1567#issuecomment-711124309


   # [Codecov](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567?src=pr&el=h1) Report
   > Merging [#1567](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567?src=pr&el=desc) into [master](https://codecov.io/gh/apache/shardingsphere-elasticjob/commit/8fdd64868b0dea21d9d4770968412d49b8fea33b?el=desc) will **decrease** coverage by `0.01%`.
   > The diff coverage is `72.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567/graphs/tree.svg?width=650&height=150&src=pr&token=8ZMVc4Yo4Z)](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #1567      +/-   ##
   ============================================
   - Coverage     85.39%   85.38%   -0.02%     
     Complexity      103      103              
   ============================================
     Files           250      250              
     Lines          5688     5684       -4     
     Branches        882      880       -2     
   ============================================
   - Hits           4857     4853       -4     
   + Misses          515      513       -2     
   - Partials        316      318       +2     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [.../elasticjob/infra/spi/ElasticJobServiceLoader.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1pbmZyYS9lbGFzdGljam9iLWluZnJhLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZWxhc3RpY2pvYi9pbmZyYS9zcGkvRWxhc3RpY0pvYlNlcnZpY2VMb2FkZXIuamF2YQ==) | `58.33% <41.66%> (+11.45%)` | `0.00 <0.00> (ø)` | |
   | [...asticjob/error/handler/JobErrorHandlerFactory.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1lcnJvci1oYW5kbGVyL2VsYXN0aWNqb2ItZXJyb3ItaGFuZGxlci10eXBlL2VsYXN0aWNqb2ItZXJyb3ItaGFuZGxlci1nZW5lcmFsL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9lbGFzdGljam9iL2Vycm9yL2hhbmRsZXIvSm9iRXJyb3JIYW5kbGVyRmFjdG9yeS5qYXZh) | `100.00% <100.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...asticjob/executor/item/JobItemExecutorFactory.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1leGVjdXRvci9lbGFzdGljam9iLWV4ZWN1dG9yLWtlcm5lbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZWxhc3RpY2pvYi9leGVjdXRvci9pdGVtL0pvYkl0ZW1FeGVjdXRvckZhY3RvcnkuamF2YQ==) | `100.00% <100.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...a/handler/sharding/JobShardingStrategyFactory.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1pbmZyYS9lbGFzdGljam9iLWluZnJhLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZWxhc3RpY2pvYi9pbmZyYS9oYW5kbGVyL3NoYXJkaW5nL0pvYlNoYXJkaW5nU3RyYXRlZ3lGYWN0b3J5LmphdmE=) | `100.00% <100.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...r/threadpool/JobExecutorServiceHandlerFactory.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1pbmZyYS9lbGFzdGljam9iLWluZnJhLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZWxhc3RpY2pvYi9pbmZyYS9oYW5kbGVyL3RocmVhZHBvb2wvSm9iRXhlY3V0b3JTZXJ2aWNlSGFuZGxlckZhY3RvcnkuamF2YQ==) | `100.00% <100.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...cjob/infra/listener/ElasticJobListenerFactory.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1pbmZyYS9lbGFzdGljam9iLWluZnJhLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZWxhc3RpY2pvYi9pbmZyYS9saXN0ZW5lci9FbGFzdGljSm9iTGlzdGVuZXJGYWN0b3J5LmphdmE=) | `100.00% <100.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...ener/AbstractDistributeOnceElasticJobListener.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1saXRlL2VsYXN0aWNqb2ItbGl0ZS1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9lbGFzdGljam9iL2xpdGUvYXBpL2xpc3RlbmVyL0Fic3RyYWN0RGlzdHJpYnV0ZU9uY2VFbGFzdGljSm9iTGlzdGVuZXIuamF2YQ==) | `74.07% <0.00%> (-7.41%)` | `0.00% <0.00%> (ø%)` | |
   | [...e/shardingsphere/elasticjob/infra/env/IpUtils.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1pbmZyYS9lbGFzdGljam9iLWluZnJhLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZWxhc3RpY2pvYi9pbmZyYS9lbnYvSXBVdGlscy5qYXZh) | `60.00% <0.00%> (-4.62%)` | `0.00% <0.00%> (ø%)` | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567?src=pr&el=footer). Last update [8fdd648...eda2b19](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1567?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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:
users@infra.apache.org