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/07/15 15:55:03 UTC

[GitHub] [shardingsphere] hongfuli opened a new pull request #6361: support absolute path for conf dir

hongfuli opened a new pull request #6361:
URL: https://github.com/apache/shardingsphere/pull/6361


   And fix the problem of loading logback.xml
   
   issue: #6324
   
   Fixes  #6324
   
   Changes proposed in this pull request:
   -
   -
   -
   


----------------------------------------------------------------
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] xbkaishui commented on pull request #6361: support absolute path for conf dir

Posted by GitBox <gi...@apache.org>.
xbkaishui commented on pull request #6361:
URL: https://github.com/apache/shardingsphere/pull/6361#issuecomment-659299570


   kindly remainder, there are three type of start script, one for window , one for linux , and one for docker. please change them all. 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.

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



[GitHub] [shardingsphere] xbkaishui commented on a change in pull request #6361: support absolute path for conf dir

Posted by GitBox <gi...@apache.org>.
xbkaishui commented on a change in pull request #6361:
URL: https://github.com/apache/shardingsphere/pull/6361#discussion_r455651539



##########
File path: shardingsphere-proxy/shardingsphere-proxy-common/src/main/java/org/apache/shardingsphere/proxy/config/ShardingConfigurationLoader.java
##########
@@ -54,8 +53,8 @@
      */
     public ShardingConfiguration load(final String path) throws IOException {
         Collection<String> schemaNames = new HashSet<>();
-        YamlProxyServerConfiguration serverConfig = loadServerConfiguration(getResourceFile(path + "/" + SERVER_CONFIG_FILE));
-        File configPath = getResourceFile(path);

Review comment:
       why remove this, the bug is caused by script, this is usable when run the proxy in idea and load the absolute path 

##########
File path: shardingsphere-proxy/shardingsphere-proxy-common/src/main/java/org/apache/shardingsphere/proxy/config/ShardingConfigurationLoader.java
##########
@@ -69,14 +68,6 @@ public ShardingConfiguration load(final String path) throws IOException {
         return new ShardingConfiguration(serverConfig, ruleConfigurationMap);
     }
     
-    private File getResourceFile(final String path) {
-        URL url = ShardingConfigurationLoader.class.getResource(path);

Review comment:
       this is compatibility with orginal classloader logic 




----------------------------------------------------------------
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] tristaZero commented on pull request #6361: support absolute path for conf dir

Posted by GitBox <gi...@apache.org>.
tristaZero commented on pull request #6361:
URL: https://github.com/apache/shardingsphere/pull/6361#issuecomment-659267483


   @hongfuli Hi Thanks for your PR!
   Since @xbkaishui did something with it, I suggest it is better to get @xbkaishui a look.


----------------------------------------------------------------
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] hongfuli closed pull request #6361: support absolute path for conf dir

Posted by GitBox <gi...@apache.org>.
hongfuli closed pull request #6361:
URL: https://github.com/apache/shardingsphere/pull/6361


   


----------------------------------------------------------------
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] xbkaishui commented on pull request #6361: support absolute path for conf dir

Posted by GitBox <gi...@apache.org>.
xbkaishui commented on pull request #6361:
URL: https://github.com/apache/shardingsphere/pull/6361#issuecomment-659453352


   Ok, thanks for your bug report. will fix this tmr


----------------------------------------------------------------
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] hongfuli commented on pull request #6361: support absolute path for conf dir

Posted by GitBox <gi...@apache.org>.
hongfuli commented on pull request #6361:
URL: https://github.com/apache/shardingsphere/pull/6361#issuecomment-659445577


   > kindly remainder, there are three type of start script, one for window , one for linux , and one for docker. please change them all. thanks
   
   I don't have windows environment to test, this script problem is still need you to improve, thanks.
   
   I close the PR.


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