You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@kyuubi.apache.org by GitBox <gi...@apache.org> on 2022/12/13 18:07:29 UTC
[GitHub] [incubator-kyuubi] dnskr opened a new pull request, #3976: Helm chart improvements
dnskr opened a new pull request, #3976:
URL: https://github.com/apache/incubator-kyuubi/pull/3976
### _Why are the changes needed?_
The changes are needed to improve helm chart usability.
The PR introduces a few enhancements (one per commit) to make Kyuubi helm chart more friendly to be used.
### _How was this patch tested?_
- [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible
- [ ] Add screenshots for manual tests if appropriate
- [ ] [Run test](https://kyuubi.apache.org/docs/latest/develop_tools/testing.html#running-tests) locally before make a 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.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] dnskr commented on a diff in pull request #3976: Helm chart improvements
Posted by GitBox <gi...@apache.org>.
dnskr commented on code in PR #3976:
URL: https://github.com/apache/incubator-kyuubi/pull/3976#discussion_r1048241320
##########
docker/helm/templates/kyuubi-configmap.yaml:
##########
@@ -18,7 +18,7 @@
apiVersion: v1
kind: ConfigMap
metadata:
- name: kyuubi-defaults
+ name: {{ .Release.Name }}-kyuubi-defaults
Review Comment:
I would like to keep `{{ .Release.Name }}-kyuubi-defaults` name for now because it represents that `ConfigMap` contains `kyuubi-defaults.conf`.
However, I want to add `kyuubi-env.sh` in next PR , so I'll change name to `{{ .Release.Name }}` similar to other resources.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] ulysses-you commented on pull request #3976: Helm chart improvements
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on PR #3976:
URL: https://github.com/apache/incubator-kyuubi/pull/3976#issuecomment-1350766769
thanks, merging to master
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] ulysses-you commented on a diff in pull request #3976: Helm chart improvements
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on code in PR #3976:
URL: https://github.com/apache/incubator-kyuubi/pull/3976#discussion_r1047921750
##########
docker/helm/templates/kyuubi-configmap.yaml:
##########
@@ -18,7 +18,7 @@
apiVersion: v1
kind: ConfigMap
metadata:
- name: kyuubi-defaults
+ name: {{ .Release.Name }}-kyuubi-defaults
Review Comment:
{{ .Release.Name }}-configmap
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] ulysses-you commented on pull request #3976: Helm chart improvements
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on PR #3976:
URL: https://github.com/apache/incubator-kyuubi/pull/3976#issuecomment-1350359146
cc @hddong
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] pan3793 commented on pull request #3976: Helm chart improvements
Posted by GitBox <gi...@apache.org>.
pan3793 commented on PR #3976:
URL: https://github.com/apache/incubator-kyuubi/pull/3976#issuecomment-1350883820
Thanks @dnskr
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] ulysses-you commented on a diff in pull request #3976: Helm chart improvements
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on code in PR #3976:
URL: https://github.com/apache/incubator-kyuubi/pull/3976#discussion_r1048244827
##########
docker/helm/templates/kyuubi-configmap.yaml:
##########
@@ -18,7 +18,7 @@
apiVersion: v1
kind: ConfigMap
metadata:
- name: kyuubi-defaults
+ name: {{ .Release.Name }}-kyuubi-defaults
Review Comment:
got it, look forward to your next 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.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] pan3793 commented on pull request #3976: Helm chart improvements
Posted by GitBox <gi...@apache.org>.
pan3793 commented on PR #3976:
URL: https://github.com/apache/incubator-kyuubi/pull/3976#issuecomment-1350238487
@dnskr thanks for your effort, it's awesome to make the commit history so clear, but if we directly merge it(because it uses squash), it will lost the commit history, would you mind opening PR one by one?
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] dnskr commented on pull request #3976: Helm chart improvements
Posted by GitBox <gi...@apache.org>.
dnskr commented on PR #3976:
URL: https://github.com/apache/incubator-kyuubi/pull/3976#issuecomment-1350779827
> @dnskr thanks for your effort, it's awesome to make the commit history so clear, but if we directly merge it(our merge tool forcibly uses squash), commit history will be lost, would you mind opening PR one by one?
@pan3793 I created several commits just to make review process easier. Squashed commit contains all commit messages, so looks good for me. For next PRs I'll do it more granular (one change per 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.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] ulysses-you closed pull request #3976: Helm chart improvements
Posted by GitBox <gi...@apache.org>.
ulysses-you closed pull request #3976: Helm chart improvements
URL: https://github.com/apache/incubator-kyuubi/pull/3976
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org