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/05/23 04:39:58 UTC

[GitHub] [shardingsphere] cherrylzhao opened a new pull request #5765: download the wait-for-it.sh from github

cherrylzhao opened a new pull request #5765:
URL: https://github.com/apache/shardingsphere/pull/5765


   for license issue, remove the wait-for-it.sh, download it from GitHub in docile
   


----------------------------------------------------------------
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] kezhenxu94 commented on a change in pull request #5765: download the wait-for-it.sh from github

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



##########
File path: shardingsphere-integration-test/shardingsphere-proxy-docker-build/Dockerfile
##########
@@ -20,6 +20,6 @@ FROM java:8
 
 ARG APP_NAME
 ADD target/${APP_NAME}.tar.gz /opt
-ADD src/main/resources/tool/wait-for-it.sh /opt
+ADD https://github.com/vishnubob/wait-for-it/blob/master/wait-for-it.sh /opt

Review comment:
       This link seems to be in html format, you may want to use https://raw.githubusercontent.com/vishnubob/wait-for-it/master/wait-for-it.sh ?




----------------------------------------------------------------
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] cherrylzhao commented on pull request #5765: download the wait-for-it.sh from github

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


   > BTW, @cherrylzhao do you think it is required to wget this third part file? What concerns me a bit is that we can not ensure this file exists there all the time, right?
   
   I have no better idea currently, it need this shell to control the order of docker container 
   
   


----------------------------------------------------------------
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] kezhenxu94 commented on pull request #5765: download the wait-for-it.sh from github

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


   > > BTW, @cherrylzhao do you think it is required to wget this third part file? What concerns me a bit is that we can not ensure this file exists there all the time, right?
   > 
   > I have no better idea currently, it need this shell to control the order of docker container
   
   @cherrylzhao after checking the previous pull request, if I understand correctly, you're trying to determine the startup order and wait for some services to be ready, we have the same requirement in SkyWalking too, and we finally did it by using docker-compose version 2 format, where you can define the health check and dependencies between services, take this as an example
   
   https://github.com/apache/skywalking/blob/a12ecea4494ff9e00f81d82b4ba9119039d0e011/test/e2e/e2e-test/docker/base-compose.yml#L61-L65
   
   https://github.com/apache/skywalking/blob/a12ecea4494ff9e00f81d82b4ba9119039d0e011/test/e2e/e2e-test/docker/simple/jdk/docker-compose.yml#L36-L38
   
   hope it helps


----------------------------------------------------------------
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] cherrylzhao commented on pull request #5765: download the wait-for-it.sh from github

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


   
   > Yes agree, downloading from third-party repo may break someday if they're gone, or even when they have breaking changes, since you're always downloading the latest (master branch) codes, but for test purpose, I think it's OK to do so, yet it's better to lock the version, say https://raw.githubusercontent.com/vishnubob/wait-for-it/c096cface5fbd9f2d6b037391dfecae6fde1362e/wait-for-it.sh for example
   
   agree with it, raw.github file with version make sense to me
   
   


----------------------------------------------------------------
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] cherrylzhao closed pull request #5765: download the wait-for-it.sh from github

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


   


----------------------------------------------------------------
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] kimmking commented on pull request #5765: download the wait-for-it.sh from github

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


   Another choice is post to offical website and refered url like:
   > http://shardingsphere.apache.org/tools/xx.sh


----------------------------------------------------------------
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] cherrylzhao commented on pull request #5765: download the wait-for-it.sh from github

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


   
   > @cherrylzhao after checking the previous pull request, if I understand correctly, you're trying to determine the startup order and wait for some services to be ready, we have the same requirement in SkyWalking too, and we finally did it by using docker-compose version 2 format, where you can define the health check and dependencies between services, take this as an example
   > 
   > https://github.com/apache/skywalking/blob/a12ecea4494ff9e00f81d82b4ba9119039d0e011/test/e2e/e2e-test/docker/base-compose.yml#L61-L65
   > 
   > https://github.com/apache/skywalking/blob/a12ecea4494ff9e00f81d82b4ba9119039d0e011/test/e2e/e2e-test/docker/simple/jdk/docker-compose.yml#L36-L38
   > 
   > hope it helps
   
   LGTM, I will consider about downgrading the docker-compose if possible


----------------------------------------------------------------
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] kezhenxu94 commented on pull request #5765: download the wait-for-it.sh from github

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


   The commit is reverted and this could be closed now


----------------------------------------------------------------
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 #5765: download the wait-for-it.sh from github

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


   Hi @kezhenxu94  
   I contacted @cherrylzhao just now and he was kind to fix it here. If available, could you give it a quick look? Thanks. 
   
   BTW, @cherrylzhao do you think it is required to wget this third part file? What concerns me a bit is that we can not ensure this file exists there all the time, right?


----------------------------------------------------------------
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] kezhenxu94 commented on pull request #5765: download the wait-for-it.sh from github

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


   > BTW, @cherrylzhao do you think it is required to wget this third part file? What concerns me a bit is that we can not ensure this file exists there all the time, right?
   
   Yes agree, downloading from third-party repo may break someday if they're gone, or even when they have breaking changes, since you're always downloading the latest (master branch) codes, but for test purpose, I think it's OK to do so, yet it's better to lock the version, say https://raw.githubusercontent.com/vishnubob/wait-for-it/c096cface5fbd9f2d6b037391dfecae6fde1362e/wait-for-it.sh for example


----------------------------------------------------------------
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] codecov-commenter commented on pull request #5765: download the wait-for-it.sh from github

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #5765:
URL: https://github.com/apache/shardingsphere/pull/5765#issuecomment-632987200


   # [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/5765?src=pr&el=h1) Report
   > Merging [#5765](https://codecov.io/gh/apache/shardingsphere/pull/5765?src=pr&el=desc) into [master](https://codecov.io/gh/apache/shardingsphere/commit/d07e6aa3f542432b3f807e61db95d724b3b05681&el=desc) will **not change** coverage.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/shardingsphere/pull/5765/graphs/tree.svg?width=650&height=150&src=pr&token=ZvlXpWa7so)](https://codecov.io/gh/apache/shardingsphere/pull/5765?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff            @@
   ##             master    #5765   +/-   ##
   =========================================
     Coverage     53.04%   53.04%           
     Complexity      437      437           
   =========================================
     Files          1178     1178           
     Lines         20721    20721           
     Branches       3736     3736           
   =========================================
     Hits          10991    10991           
     Misses         9059     9059           
     Partials        671      671           
   ```
   
   
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/shardingsphere/pull/5765?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/pull/5765?src=pr&el=footer). Last update [d07e6aa...2f5bd06](https://codecov.io/gh/apache/shardingsphere/pull/5765?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