You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@kvrocks.apache.org by "xxaier (via GitHub)" <gi...@apache.org> on 2023/05/25 03:42:49 UTC
[GitHub] [incubator-kvrocks] xxaier opened a new pull request, #1474: docker can use env conf
xxaier opened a new pull request, #1474:
URL: https://github.com/apache/incubator-kvrocks/pull/1474
* docker can use env conf
* add docker/build.sh for build docker in local
* add docker-compose.yml
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] xxaier commented on a diff in pull request #1474: docker can use env conf
Posted by "xxaier (via GitHub)" <gi...@apache.org>.
xxaier commented on code in PR #1474:
URL: https://github.com/apache/incubator-kvrocks/pull/1474#discussion_r1205664672
##########
Dockerfile:
##########
@@ -57,10 +57,9 @@ VOLUME /var/lib/kvrocks
RUN chown kvrocks:kvrocks /var/lib/kvrocks
-COPY ./LICENSE ./NOTICE ./DISCLAIMER ./
+COPY ./docker/run.sh ./LICENSE ./NOTICE ./DISCLAIMER ./
COPY ./licenses ./licenses
COPY ./kvrocks.conf /var/lib/kvrocks/
-
EXPOSE 6666:6666
-ENTRYPOINT ["./bin/kvrocks", "-c", "/var/lib/kvrocks/kvrocks.conf", "--dir", "/var/lib/kvrocks", "--pidfile", "/var/run/kvrocks/kvrocks.pid"]
+ENTRYPOINT ["./run.sh"]
Review Comment:
see https://stackoverflow.com/questions/37904682/how-do-i-use-docker-environment-variable-in-entrypoint-array
hard to do this
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] xxaier commented on a diff in pull request #1474: docker can use env conf
Posted by "xxaier (via GitHub)" <gi...@apache.org>.
xxaier commented on code in PR #1474:
URL: https://github.com/apache/incubator-kvrocks/pull/1474#discussion_r1205664665
##########
Dockerfile:
##########
@@ -57,10 +57,9 @@ VOLUME /var/lib/kvrocks
RUN chown kvrocks:kvrocks /var/lib/kvrocks
-COPY ./LICENSE ./NOTICE ./DISCLAIMER ./
+COPY ./docker/run.sh ./LICENSE ./NOTICE ./DISCLAIMER ./
COPY ./licenses ./licenses
COPY ./kvrocks.conf /var/lib/kvrocks/
-
EXPOSE 6666:6666
-ENTRYPOINT ["./bin/kvrocks", "-c", "/var/lib/kvrocks/kvrocks.conf", "--dir", "/var/lib/kvrocks", "--pidfile", "/var/run/kvrocks/kvrocks.pid"]
+ENTRYPOINT ["./run.sh"]
Review Comment:
see https://stackoverflow.com/questions/37904682/how-do-i-use-docker-environment-variable-in-entrypoint-array
hard to do this
##########
Dockerfile:
##########
@@ -57,10 +57,9 @@ VOLUME /var/lib/kvrocks
RUN chown kvrocks:kvrocks /var/lib/kvrocks
-COPY ./LICENSE ./NOTICE ./DISCLAIMER ./
+COPY ./docker/run.sh ./LICENSE ./NOTICE ./DISCLAIMER ./
COPY ./licenses ./licenses
COPY ./kvrocks.conf /var/lib/kvrocks/
-
EXPOSE 6666:6666
-ENTRYPOINT ["./bin/kvrocks", "-c", "/var/lib/kvrocks/kvrocks.conf", "--dir", "/var/lib/kvrocks", "--pidfile", "/var/run/kvrocks/kvrocks.pid"]
+ENTRYPOINT ["./run.sh"]
Review Comment:
see https://stackoverflow.com/questions/37904682/how-do-i-use-docker-environment-variable-in-entrypoint-array
hard to do this
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] torwig commented on pull request #1474: docker can use env conf
Posted by "torwig (via GitHub)" <gi...@apache.org>.
torwig commented on PR #1474:
URL: https://github.com/apache/incubator-kvrocks/pull/1474#issuecomment-1562421234
@xxaier Maybe, we can implement a feature to pass options via an env variable similar to `REDIS_ARGS`. However, personally, I don't like the trick with a bash script to 'translate` env variables to Kvrocks options.
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] git-hulk closed pull request #1474: docker can use env conf
Posted by "git-hulk (via GitHub)" <gi...@apache.org>.
git-hulk closed pull request #1474: docker can use env conf
URL: https://github.com/apache/incubator-kvrocks/pull/1474
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on pull request #1474: docker can use env conf
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on PR #1474:
URL: https://github.com/apache/incubator-kvrocks/pull/1474#issuecomment-1562253594
I acknowledge your idea to pass env vars as options, but IMHO the shell script composed by `sed` and some other text substitution tools looks terrible to me since it is more like workaround than a real solution.
BTW, shell scripts are hard to maintain and be general-purpose enough. Our development scripts are all written in python, refer to `x.py`.
For [docker/build.sh](https://github.com/apache/incubator-kvrocks/pull/1474/files#diff-efe74abc7213e95246105b4a7b029557ef16ff997e9cc13eadcfffa18e6757c0), I think we do not need the script. You can learn how our community build and release docker images from https://kvrocks.apache.org/community/create-a-release .
And for [docker/docker-compose.yml](https://github.com/apache/incubator-kvrocks/pull/1474/files#diff-423deb13b7c401b1a7f41ee91c77f722e11d2f317d6a66b546524e8a04cc8b03), I think we do not need an example compose file, and I really do not know why these arguments are chosen and are suitable for various kvrocks users, e.g. `mem_limit: 2G` `memswap_limit: -1`.
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] xxaier commented on pull request #1474: docker can use env conf
Posted by "xxaier (via GitHub)" <gi...@apache.org>.
xxaier commented on PR #1474:
URL: https://github.com/apache/incubator-kvrocks/pull/1474#issuecomment-1562471804
<img width="1918" alt="image" src="https://github.com/apache/incubator-kvrocks/assets/130831741/e96211b3-af6b-4336-8410-01d3adfd6731">
I changed code, read command line args from KVROCKS_ARGS
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on pull request #1474: docker can use env conf
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on PR #1474:
URL: https://github.com/apache/incubator-kvrocks/pull/1474#issuecomment-1562421792
> @xxaier Maybe, we can implement a feature to pass options via an env variable similar to `REDIS_ARGS`. However, personally, I don't like the trick with a bash script to 'translate` env variables to Kvrocks options.
+1. `REDIS_ARGS` looks fine 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.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] xxaier commented on pull request #1474: docker can use env conf
Posted by "xxaier (via GitHub)" <gi...@apache.org>.
xxaier commented on PR #1474:
URL: https://github.com/apache/incubator-kvrocks/pull/1474#issuecomment-1562400477
> I acknowledge your idea to pass env vars as options, but IMHO the shell script composed by `sed` and some other text substitution tools looks terrible to me since it is more like workaround than a real solution.
>
> BTW, shell scripts are hard to maintain and be general-purpose enough. Our development scripts are all written in python, refer to `x.py`.
>
> For [docker/build.sh](https://github.com/apache/incubator-kvrocks/pull/1474/files#diff-efe74abc7213e95246105b4a7b029557ef16ff997e9cc13eadcfffa18e6757c0), I think we do not need the script. You can learn how our community build and release docker images from https://kvrocks.apache.org/community/create-a-release .
>
> And for [docker/docker-compose.yml](https://github.com/apache/incubator-kvrocks/pull/1474/files#diff-423deb13b7c401b1a7f41ee91c77f722e11d2f317d6a66b546524e8a04cc8b03), I think we do not need an example compose file, and I really do not know why these arguments are chosen and are suitable for various kvrocks users, e.g. `mem_limit: 2G` `memswap_limit: -1`.
The reason why I don't use python is that in order to keep the size of the final docker container from growing, the final docker image will not contain the python runtime.
The reason I added build.sh is that when developing and debugging docker, there will be a need for local construction, such as building a database based on kvrocks+topling https://github.com/apache/incubator-kvrocks/pull/1206.
I think local development and building docker debugging is also a common scenario, and it is very troublesome to manually enter commands every time.
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] git-hulk commented on pull request #1474: docker can use env conf
Posted by "git-hulk (via GitHub)" <gi...@apache.org>.
git-hulk commented on PR #1474:
URL: https://github.com/apache/incubator-kvrocks/pull/1474#issuecomment-1588418529
Close the PR due to inactive, feel free to reopen if you like.
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] git-hulk commented on a diff in pull request #1474: docker can use env conf
Posted by "git-hulk (via GitHub)" <gi...@apache.org>.
git-hulk commented on code in PR #1474:
URL: https://github.com/apache/incubator-kvrocks/pull/1474#discussion_r1205353255
##########
Dockerfile:
##########
@@ -57,10 +57,9 @@ VOLUME /var/lib/kvrocks
RUN chown kvrocks:kvrocks /var/lib/kvrocks
-COPY ./LICENSE ./NOTICE ./DISCLAIMER ./
+COPY ./docker/run.sh ./LICENSE ./NOTICE ./DISCLAIMER ./
COPY ./licenses ./licenses
COPY ./kvrocks.conf /var/lib/kvrocks/
-
EXPOSE 6666:6666
-ENTRYPOINT ["./bin/kvrocks", "-c", "/var/lib/kvrocks/kvrocks.conf", "--dir", "/var/lib/kvrocks", "--pidfile", "/var/run/kvrocks/kvrocks.pid"]
+ENTRYPOINT ["./run.sh"]
Review Comment:
Can we only add the KVROCKS_ARGS for the custom configuration and leave the others as they should(remove the run.sh and docker-compose.yml)?
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] xxaier commented on pull request #1474: docker can use env conf
Posted by "xxaier (via GitHub)" <gi...@apache.org>.
xxaier commented on PR #1474:
URL: https://github.com/apache/incubator-kvrocks/pull/1474#issuecomment-1562233080
docker run apache/kvrocks --some-key some-value --other-key other-value ....
will run , but if you write docker-compose.yml and need read env for a .env, how to write ? change cmd ? Such an operation is very unintuitive.
These bash scripts can be run on all platforms with docker and bash installed.
Configuring docker through environment variables I think is a very common practice
for exampe nginx : https://hub.docker.com/_/nginx
<img width="818" alt="image" src="https://github.com/apache/incubator-kvrocks/assets/130831741/f5e9aab2-33ea-4f81-b9a4-4808fd6780bf">
for example postgres : https://hub.docker.com/_/postgres
<img width="929" alt="image" src="https://github.com/apache/incubator-kvrocks/assets/130831741/9509c93d-18bc-47c9-a00e-07945627248d">
for example redis : https://hub.docker.com/_/redis
<img width="1050" alt="image" src="https://github.com/apache/incubator-kvrocks/assets/130831741/dc03feae-832d-418b-9895-0e665e96ba0d">
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org