You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@fluss.apache.org by "nhuantho (via GitHub)" <gi...@apache.org> on 2026/03/14 08:22:07 UTC
[PR] [helm] Restructure helm chart of Fluss [fluss]
nhuantho opened a new pull request, #2871:
URL: https://github.com/apache/fluss/pull/2871
<!--
*Thank you very much for contributing to Fluss - we are happy that you want to help us improve Fluss. To help the community review your contribution in the best possible way, please go through the checklist below, which will get the contribution into a shape in which it can be best reviewed.*
## Contribution Checklist
- Make sure that the pull request corresponds to a [GitHub issue](https://github.com/apache/fluss/issues). Exceptions are made for typos in JavaDoc or documentation files, which need no issue.
- Name the pull request in the format "[component] Title of the pull request", where *[component]* should be replaced by the name of the component being changed. Typically, this corresponds to the component label assigned to the issue (e.g., [kv], [log], [client], [flink]). Skip *[component]* if you are unsure about which is the best component.
- Fill out the template below to describe the changes contributed by the pull request. That will give reviewers the context they need to do the review.
- Make sure that the change passes the automated tests, i.e., `mvn clean verify` passes.
- Each pull request should address only one issue, not mix up code from multiple issues.
**(The sections below can be removed for hotfixes or typos)**
-->
### Purpose
<!-- Linking this pull request to the issue -->
Linked issue: close https://github.com/apache/fluss/issues/2865
<!-- What is the purpose of the change -->
### Brief change log
* Restructured `values.yaml`
* coordinator -> coordinatorServer (https://fluss.apache.org/docs/concepts/architecture/).
* tablet -> tabletServer (https://fluss.apache.org/docs/concepts/architecture/).
* Move resource of coordinatorServer and tabletServer to inside themselves.
* listeners -> ports.
* storage -> persistence.
* Created `values.schema.json`: schema of `values.yaml` and standard to valid `value.yaml`.
* Created `values.schema.schema.json`: schema of `values.schema.json` and standard to valid `values.schema.json`.
* Created `tools/ci/helm_ci/validate_helm.py` to validate helm.
* Add ci to validate helm.
* Add ignore python in `.gitignore`
### Tests
<!-- List UT and IT cases to verify this change -->
### API and Format
<!-- Does this change affect API or storage format -->
### Documentation
<!-- Does this change introduce a new feature -->
--
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@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [helm] Restructure helm chart of Fluss [fluss]
Posted by "affo (via GitHub)" <gi...@apache.org>.
affo commented on code in PR #2871:
URL: https://github.com/apache/fluss/pull/2871#discussion_r2939724755
##########
.github/workflows/helm-chart.yaml:
##########
@@ -44,6 +44,17 @@ jobs:
- name: "Checkout code"
uses: actions/checkout@v6
+ - name: Set up Python
+ uses: actions/setup-python@v5
+ with:
+ python-version: "3.11"
+
+ - name: Install dependencies
+ run: pip install jsonschema==4.26.0 PyYAML==6.0.3
+
+ - name: Run unstable test reporter
Review Comment:
```suggestion
- name: Validate values
```
##########
.github/workflows/helm-chart.yaml:
##########
@@ -44,6 +44,17 @@ jobs:
- name: "Checkout code"
uses: actions/checkout@v6
+ - name: Set up Python
+ uses: actions/setup-python@v5
+ with:
+ python-version: "3.11"
Review Comment:
```suggestion
python-version: "3.12"
```
Just better to point to that one if we don't break anything
##########
tools/ci/helm_ci/validate_helm.py:
##########
@@ -0,0 +1,89 @@
+# Licensed to the Apache Software Foundation (ASF) under one
Review Comment:
Why do we need this script ?
Isn't `helm lint` already picking `values.schema.json` for validation?
Is this only for the meta schema?
##########
.github/workflows/helm-chart.yaml:
##########
@@ -44,6 +44,17 @@ jobs:
- name: "Checkout code"
uses: actions/checkout@v6
+ - name: Set up Python
+ uses: actions/setup-python@v5
+ with:
+ python-version: "3.11"
+
+ - name: Install dependencies
+ run: pip install jsonschema==4.26.0 PyYAML==6.0.3
Review Comment:
I would favor a `requirements.txt` under `tools/ci/helm_ci`.
It would make it easier to validate locally.
The command would become `pip install -r tools/ci/helm_ci/requirements.txt`
--
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@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [helm] Restructure helm chart of Fluss [fluss]
Posted by "nhuantho (via GitHub)" <gi...@apache.org>.
nhuantho commented on PR #2871:
URL: https://github.com/apache/fluss/pull/2871#issuecomment-4062666939
@swuferhong can you review for 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@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [helm] Restructure helm chart of Fluss [fluss]
Posted by "nhuantho (via GitHub)" <gi...@apache.org>.
nhuantho commented on code in PR #2871:
URL: https://github.com/apache/fluss/pull/2871#discussion_r2940815033
##########
tools/ci/helm_ci/validate_helm.py:
##########
@@ -0,0 +1,89 @@
+# Licensed to the Apache Software Foundation (ASF) under one
Review Comment:
@affo, from my side, when everyone updated helm. Could they please explain why it needs to be updated and describe the changes in the schema?
For example:
<img width="908" height="249" alt="image" src="https://github.com/user-attachments/assets/4f1cf214-51df-437c-a042-13f3ceec6460" />
--
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@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [helm] Restructure helm chart of Fluss [fluss]
Posted by "nhuantho (via GitHub)" <gi...@apache.org>.
nhuantho commented on code in PR #2871:
URL: https://github.com/apache/fluss/pull/2871#discussion_r2940928345
##########
.github/workflows/helm-chart.yaml:
##########
@@ -44,6 +44,17 @@ jobs:
- name: "Checkout code"
uses: actions/checkout@v6
+ - name: Set up Python
+ uses: actions/setup-python@v5
+ with:
+ python-version: "3.11"
Review Comment:
I want to unify the Python version with `.github/workflows/unstable-test-reporter.yaml`
--
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@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [helm] Restructure helm chart of Fluss [fluss]
Posted by "affo (via GitHub)" <gi...@apache.org>.
affo commented on PR #2871:
URL: https://github.com/apache/fluss/pull/2871#issuecomment-6262310787
@polyzos thanks for hopping in!
@nhuantho as said in the past, I would keep the schema validation part and avoid the restructuring, at least not as part of this PR.
@nhuantho for the schema validation part, I think we need to iterate whether it makes sense to add the python script for validation instead of something "helm-native" whether it is available.
--
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@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [helm] Restructure helm chart of Fluss [fluss]
Posted by "nhuantho (via GitHub)" <gi...@apache.org>.
nhuantho commented on PR #2871:
URL: https://github.com/apache/fluss/pull/2871#issuecomment-4062661931
@affo Can you review for 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@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [helm] Restructure helm chart of Fluss [fluss]
Posted by "nhuantho (via GitHub)" <gi...@apache.org>.
nhuantho commented on PR #2871:
URL: https://github.com/apache/fluss/pull/2871#issuecomment-4068282139
@affo, I will answer some confusion:
* I think it will confuse users who use Helm
```
# @schema
# type: object
# additionalProperties: false
# @schema
```
* https://github.com/dadav/helm-schema is not fully open source, and it is easy for us to build unit tests for Helm.
* https://github.com/norwoodj/helm-docs is also not fully open source, and I think we can easily build and generate docs.
Focus pr:
* We have the first verified value change to avoid affecting the helm -> https://github.com/apache/fluss/pull/2472
* Suggest defining value with a standard (airflow).
* Build a schema and describe all values of helm, and the first step for auto-generating helm docs.
--
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@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [helm] Restructure helm chart of Fluss [fluss]
Posted by "affo (via GitHub)" <gi...@apache.org>.
affo commented on PR #2871:
URL: https://github.com/apache/fluss/pull/2871#issuecomment-4067134630
@nhuantho as a side note, one can already structure schema annotations by adding comments prefixed with `--` to pair with `helm-docs` which is a tool we may add for generating the docs https://github.com/norwoodj/helm-docs
--
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@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [helm] Restructure helm chart of Fluss [fluss]
Posted by "affo (via GitHub)" <gi...@apache.org>.
affo commented on PR #2871:
URL: https://github.com/apache/fluss/pull/2871#issuecomment-4067128798
@nhuantho by the way, thanks for the contribution and to light the spotlight on this defect 🤝
--
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@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [helm] Restructure helm chart of Fluss [fluss]
Posted by "polyzos (via GitHub)" <gi...@apache.org>.
polyzos commented on PR #2871:
URL: https://github.com/apache/fluss/pull/2871#issuecomment-6251774665
whats the status here? should we close this or is it WIP?
--
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@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org