You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2021/04/27 02:33:17 UTC
[GitHub] [druid] asdf2014 opened a new pull request #11163: Add helm chart for Apache Druid
asdf2014 opened a new pull request #11163:
URL: https://github.com/apache/druid/pull/11163
<!-- Thanks for trying to help us make Apache Druid be the best it can be! Please fill out as much of the following information as is possible (where relevant, and remove it when irrelevant) to help make the intention and scope of this PR clear in order to ease review. -->
<!-- Please read the doc for contribution (https://github.com/apache/druid/blob/master/CONTRIBUTING.md) before making this PR. Also, once you open a PR, please _avoid using force pushes and rebasing_ since these make it difficult for reviewers to see what you've changed in response to their reviews. See [the 'If your pull request shows conflicts with master' section](https://github.com/apache/druid/blob/master/CONTRIBUTING.md#if-your-pull-request-shows-conflicts-with-master) for more details. -->
<!-- Replace XXXX with the id of the issue fixed in this PR. Remove this section if there is no corresponding issue. Don't reference the issue in the title of this pull-request. -->
<!-- If you are a committer, follow the PR action item checklist for committers:
https://github.com/apache/druid/blob/master/dev/committer-instructions.md#pr-and-issue-action-item-checklist-for-committers. -->
### Description
<!-- Describe the goal of this PR, what problem are you fixing. If there is a corresponding issue (referenced above), it's not necessary to repeat the description here, however, you may choose to keep one summary sentence. -->
<!-- Describe your patch: what did you change in code? How did you fix the problem? -->
<!-- If there are several relatively logically separate changes in this PR, create a mini-section for each of them. For example: -->
This patch added a helm chart for Apache Druid.
<!--
In each section, please describe design decisions made, including:
- Choice of algorithms
- Behavioral aspects. What configuration values are acceptable? How are corner cases and error conditions handled, such as when there are insufficient resources?
- Class organization and design (how the logic is split between classes, inheritance, composition, design patterns)
- Method organization and design (how the logic is split between methods, parameters and return types)
- Naming (class, method, API, configuration, HTTP endpoint, names of emitted metrics)
-->
<!-- It's good to describe an alternative design (or mention an alternative name) for every design (or naming) decision point and compare the alternatives with the designs that you've implemented (or the names you've chosen) to highlight the advantages of the chosen designs and names. -->
<!-- If there was a discussion of the design of the feature implemented in this PR elsewhere (e. g. a "Proposal" issue, any other issue, or a thread in the development mailing list), link to that discussion from this PR description and explain what have changed in your final design compared to your original proposal or the consensus version in the end of the discussion. If something hasn't changed since the original discussion, you can omit a detailed discussion of those aspects of the design here, perhaps apart from brief mentioning for the sake of readability of this PR description. -->
<!-- Some of the aspects mentioned above may be omitted for simple and small changes. -->
<hr>
<!-- Check the items by putting "x" in the brackets for the done things. Not all of these items apply to every PR. Remove the items which are not done or not relevant to the PR. None of the items from the checklist below are strictly necessary, but it would be very helpful if you at least self-review the PR. -->
This PR has:
- [x] been self-reviewed.
- [x] using the [concurrency checklist](https://github.com/apache/druid/blob/master/dev/code-review/concurrency.md) (Remove this item if the PR doesn't have any relation to concurrency.)
- [x] added documentation for new or modified features or behaviors.
- [x] added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
- [x] added or updated version, license, or notice information in [licenses.yaml](https://github.com/apache/druid/blob/master/dev/license.md)
- [x] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
- [x] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for [code coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md) is met.
- [x] added integration tests.
- [x] been tested in a test Druid cluster.
cc @maver1ck @AWaterColorPen
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] asdf2014 commented on pull request #11163: Add helm chart for Apache Druid
Posted by GitBox <gi...@apache.org>.
asdf2014 commented on pull request #11163:
URL: https://github.com/apache/druid/pull/11163#issuecomment-829894229
@jihoonson I see. And I agree with your suggestion. Let us follow the process of IP clearance to eliminate potential problems. I will do my best to provide answers to these questions. I have discussed it with other maintainers, and there are many improvements to it that cannot be contributed to the open source community. So, let's unify the maintenance of Druid's Helm Chart as soon as possible. Thanks a lot for your patience.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] asdf2014 commented on pull request #11163: Add helm chart for Apache Druid
Posted by GitBox <gi...@apache.org>.
asdf2014 commented on pull request #11163:
URL: https://github.com/apache/druid/pull/11163#issuecomment-829766539
Hi, @xvrl. Thanks for your comment. I agree with you, the follow-up related documents will be improved. I also mentioned before that this Druid Helm Chart is completely usable, and it has been tested several times last week.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] jihoonson edited a comment on pull request #11163: Add helm chart for Apache Druid
Posted by GitBox <gi...@apache.org>.
jihoonson edited a comment on pull request #11163:
URL: https://github.com/apache/druid/pull/11163#issuecomment-829425299
I haven't closely checked the IP clearance or other ASF processes to adopt codes from outside the project after graduation. But my 2 cents is we probably need a PMC vote for adopting because we need a plan for how to maintain and continue developing the code.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] clintropolis commented on pull request #11163: Add helm chart for Apache Druid
Posted by GitBox <gi...@apache.org>.
clintropolis commented on pull request #11163:
URL: https://github.com/apache/druid/pull/11163#issuecomment-828949027
btw, in general I think it makes sense to maintain it here, so +1 on the idea (even though I know very little about helm)
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] jihoonson commented on pull request #11163: Add helm chart for Apache Druid
Posted by GitBox <gi...@apache.org>.
jihoonson commented on pull request #11163:
URL: https://github.com/apache/druid/pull/11163#issuecomment-829885074
@asdf2014 I'm not saying we should not accept this. I'm saying this doesn't seem the right process. Have you read the [ASF IP clearance doc](https://incubator.apache.org/ip-clearance/)? The IP clearance is not just about the license. All codes in the ASF projects are owned by the ASF and the IP clearance process is required to make this part clear. The IP clearance doc explicitly says
> This is for projects and PMCs that have already been created and are receiving a code donation into an existing codebase. Any code that was developed outside of the ASF SVN repository and our public mailing lists must be processed like this, even if the external developer is already an ASF committer.
I think we should revert this PR and go through the IP clearance process because this was developed outside of ASF. Please go back to the dev thread for this issue and add more details there (not here) for people like me who don't know much about the helm chart such as
- What is the current status of the project?
- Does it reflect the most recent release of Druid?
- How is it being tested?
- Why is it best to host the helm chart in the druid repo? What alternatives have you considered?
- After migration, what is your plan for automatic testing?
I don't think this is an exhaustive list. Please add details as much as 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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] jihoonson commented on pull request #11163: Add helm chart for Apache Druid
Posted by GitBox <gi...@apache.org>.
jihoonson commented on pull request #11163:
URL: https://github.com/apache/druid/pull/11163#issuecomment-829425299
I haven't closely checked the IP clearance or other ASF processes to adopt codes from outside the project after graduation. But my 2 cents is we probably need a PMC vote for adopting.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] jihoonson commented on pull request #11163: Add helm chart for Apache Druid
Posted by GitBox <gi...@apache.org>.
jihoonson commented on pull request #11163:
URL: https://github.com/apache/druid/pull/11163#issuecomment-828936797
Hey @asdf2014, have you sorted out the potential license issue before you merge this PR? I don't see it anywhere. As Julian suggested in https://lists.apache.org/thread.html/ra93200245a27fb77ff0a1cfc17d21b7efbbb263ae422d6d2cc5ade11%40%3Cdev.druid.apache.org%3E, we may need to go through a process similar to the IP clearance for podling projects.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] asdf2014 merged pull request #11163: Add helm chart for Apache Druid
Posted by GitBox <gi...@apache.org>.
asdf2014 merged pull request #11163:
URL: https://github.com/apache/druid/pull/11163
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] asdf2014 commented on pull request #11163: Add helm chart for Apache Druid
Posted by GitBox <gi...@apache.org>.
asdf2014 commented on pull request #11163:
URL: https://github.com/apache/druid/pull/11163#issuecomment-827431081
Hi, @FrankChen021 . Thanks for your comment. I also considered whether to publish the doc, for example, put the `helm/README.md` file in the `docs/tutorials` directory. However, although this Helm Chart can deploy on the K8S cluster directly, there are still a few things that need to be completed before the document is published on the official website. For example: upgrade the Druid image version, support HDFS cluster deployment, release the Release version of Druid's Helm Chart by configuring Github Action, and etc. But these things are difficult to complete in a short period of time, so we should finish them in these follow-up PRs.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] xvrl commented on pull request #11163: Add helm chart for Apache Druid
Posted by GitBox <gi...@apache.org>.
xvrl commented on pull request #11163:
URL: https://github.com/apache/druid/pull/11163#issuecomment-829421589
@asdf2014 my understanding is that we still need to highlight were the code came from, just like we do for other Apache License 2.0 code we include or borrow.
Also, has this chart been tested? Does it still work?
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] asdf2014 commented on pull request #11163: Add helm chart for Apache Druid
Posted by GitBox <gi...@apache.org>.
asdf2014 commented on pull request #11163:
URL: https://github.com/apache/druid/pull/11163#issuecomment-828958352
Hi, @clintropolis . Thanks for your comments and support. As far as I know, the license of `helm/charts` is
Apache License 2.0. Maintaining Apache Druid’s own Helm Chart in the Apache Druid repo should not have copyright issues. I heard about the process just mentioned by @jihoonson for the first time, so I’ll look at it. In addition, Apache Superset has actually done this, and we can get more details through the link [https://github.com/apache/superset/tree/master/helm/superset](https://github.com/apache/superset/tree/master/helm/superset).
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] clintropolis commented on pull request #11163: Add helm chart for Apache Druid
Posted by GitBox <gi...@apache.org>.
clintropolis commented on pull request #11163:
URL: https://github.com/apache/druid/pull/11163#issuecomment-828946511
unrelated nitpick, i think putting this stuff in `distribution/helm/`, which is also where the Docker stuff lives, makes more sense to me than top level `helm/`
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] clintropolis commented on pull request #11163: Add helm chart for Apache Druid
Posted by GitBox <gi...@apache.org>.
clintropolis commented on pull request #11163:
URL: https://github.com/apache/druid/pull/11163#issuecomment-828945858
>Hey @asdf2014, have you sorted out the potential license issue before you merge this PR? I don't see it anywhere. As Julian suggested in https://lists.apache.org/thread.html/ra93200245a27fb77ff0a1cfc17d21b7efbbb263ae422d6d2cc5ade11%40%3Cdev.druid.apache.org%3E, we may need to go through a process similar to the IP clearance for podling projects.
I think we should consider reverting this until we get this resolved, after reading through https://incubator.apache.org/ip-clearance/, and looking at https://incubator.apache.org/ip-clearance/tika-tika-helm.html which appears to be a similar helm chart donation for another project, where I see linking to a voting thread, etc.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] asdf2014 commented on pull request #11163: Add helm chart for Apache Druid
Posted by GitBox <gi...@apache.org>.
asdf2014 commented on pull request #11163:
URL: https://github.com/apache/druid/pull/11163#issuecomment-829777938
Hi, @jihoonson. Thank you for your comments. It was previously maintained by @maver1ck @AWaterColorPen and I. Currently, https://github.com/helm/charts/tree/master/incubator/druid cannot be maintained. So, we want maintain it here, just like other Apache projects did, such as [Apache Superset](https://github.com/apache/superset/tree/master/helm/superset). Indeed, I agree with your proposal. Let's raise up a PMC vote. If it really fails to pass the vote, we can revert this PR ASAP. Could you please help initiate the vote?
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] asdf2014 commented on pull request #11163: Add helm chart for Apache Druid
Posted by GitBox <gi...@apache.org>.
asdf2014 commented on pull request #11163:
URL: https://github.com/apache/druid/pull/11163#issuecomment-828941569
Hi, @jihoonson. I didn't notice it, let me take a look. Thank you very much for reminding.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] FrankChen021 commented on pull request #11163: Add helm chart for Apache Druid
Posted by GitBox <gi...@apache.org>.
FrankChen021 commented on pull request #11163:
URL: https://github.com/apache/druid/pull/11163#issuecomment-827420445
Do we need a tutorial like the tutorial of docker ( docs/tutorials/docker.md ) ?
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org