Conversation
|
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst)
|
| **Explanation**: | ||
| `helm repo add apache-airflow https://airflow.apache.org` | ||
| - `helm repo add` is a command to add a chart repository | ||
|
|
There was a problem hiding this comment.
Hey @AgnesNM thanks for your contribution.
I think it might be better rework the existing instructions for clarity rather than adding an explanation block.
So for example, replace this:
To install this chart using Helm 3, run the following commands:
.. code-block:: bash
helm repo add apache-airflow https://airflow.apache.org
helm upgrade --install airflow apache-airflow/airflow --namespace airflow --create-namespace
With
To install this chart using Helm 3, first add the helm chart repository:
.. code-block:: bash
helm repo add apache-airflow https://airflow.apache.org
Next, install the helm chart:
.. code-block:: bash
helm upgrade --install airflow apache-airflow/airflow --namespace airflow --create-namespace
That way, we get the description saying what these commands actually do. Better continuity compared with explaining in a diff section.
| **Explanation** | ||
| - `helm upgrade` is a command to upgrade a release | ||
| - the first 'airflow' argument is the release name | ||
| - 'apache-airflow/airflow' is the Helm Chart | ||
| - the second 'airflow' argument is the namespace | ||
|
|
||
|
|
There was a problem hiding this comment.
similar to above, i would suggest that you amend existing wording if not clear already rather than adding an explanation section.
I would not document the argument syntax --- that's not our role in airflow, that is the job of the helm docs.
| **Explanation** | ||
| - `helm delete` is a command to delete a release (it was renamed to `helm uninstall`) | ||
| - the first 'airflow' argument is the release name | ||
| - the second 'airflow' argument is the namespace | ||
|
|
||
|
|
There was a problem hiding this comment.
instead, can you amend existing to replace delete with uninstall?
also, rather than explaining this
- the first 'airflow' argument is the release name
- the second 'airflow' argument is the namespace
maybe better would be to choose better names in the example in the first place... so instead of airflow (namespace) choose my-namespace, and instead of airflow (release) maybe choose my-release-name or my-airflow-release.... WDYT?
There was a problem hiding this comment.
I like this idea. It helps break up the Helm commands to identify different things with different names, instead of leaving that implicit. I would try to identify things with their types, so something like my-airflow-namespace, even though that looks a little wordy, it's more helpful for people who aren't familiar with it.
There was a problem hiding this comment.
Another thing that might help is using (making up a term here) the "options assignment" syntax, if Helm supports it:
helm repo add apache-airflow https://airflow.apache.org
helm upgrade --install=airflow apache-airflow/airflow --create-namespace --namespace=airflow
helm upgrade airflow apache-airflow/airflow --namespace=airflow
helm delete airflow --namespace=airflowBut if it doesn't then feel free to disregard.
There was a problem hiding this comment.
Yeah that's a good idea @blag but unfortunately I don't think that the helm command allows that... it's just positional
I think I found this confusing at one point in the past also
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions. |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions. |
Hello, I was wondering whether adding a little 'breakdown' of the commands on the official Helm Chart docs might help?
Especially for people who are completely new to the tool?
As in:
Installing the Chart
helm repo add apache-airflow https://airflow.apache.org helm upgrade --install airflow apache-airflow/airflow --namespace airflow --create-namespaceExplanation:
helm repo add apache-airflow https://airflow.apache.orghelm repo addis a command to add a chart repositoryhelm upgrade --install airflow apache-airflow/airflow --namespace airflow --create-namespaceExplanation
helm upgradeis a command to upgrade a releaseUpgrading the Chart
helm upgrade airflow apache-airflow/airflow --namespace airflowhelm upgradeis a command to upgrade a releaseUninstalling the Chart
helm delete airflow --namespace airflowExplanation
helm deleteis a command to delete a release (it has now been renamed tohelm uninstall)