MonkeyCanCode commented on code in PR #2001:
URL: https://github.com/apache/polaris/pull/2001#discussion_r2190637002
##########
helm/polaris/README.md.gotmpl:
##########
@@ -89,22 +71,7 @@ eval $(minikube -p minikube docker-env)
-Dquarkus.container-image.build=true
```
-### Installing the chart locally
-
-The below instructions assume a local Kubernetes cluster is running and Helm
is installed.
-
-#### Common setup
-
-Create the target namespace:
-
-```bash
-kubectl create namespace polaris
-```
-
-Create all the required resources in the `polaris` namespace. This usually
includes a Postgres
Review Comment:
So instead of running one more command, I put `--create-namespace polaris`
in the helm command instead. But if this route is more preferred, I can revert
it back.
##########
helm/polaris/README.md.gotmpl:
##########
@@ -113,67 +80,59 @@ Below are two sample deployment models for installing the
chart: one with a non-
> **These files are intended for testing purposes primarily, and may not be
> suitable for production use**.
> For production deployments, create your own values files based on the
> provided examples.
-#### Non-persistent backend
+##### Non-persistent backend
Install the chart with a non-persistent backend. From Polaris repo root:
-
```bash
helm upgrade --install --namespace polaris \
- --debug --values helm/polaris/ci/simple-values.yaml \
- polaris helm/polaris
+ --values helm/polaris/ci/simple-values.yaml \
+ polaris helm/polaris --create-namespace polaris
```
Note: if you are running the tests on a Kind cluster started with the `run.sh`
command explained
above, then you need to run `helm upgrade` as follows:
-
```bash
helm upgrade --install --namespace polaris \
- --debug --values helm/polaris/ci/simple-values.yaml \
+ --values helm/polaris/ci/simple-values.yaml \
--set=image.repository=localhost:5001/apache/polaris \
- polaris helm/polaris
+ polaris helm/polaris --create-namespace polaris
```
#### Persistent backend
> [!WARNING]
> The Postgres deployment set up in the fixtures directory is intended for
> testing purposes only and is not suitable for production use. For production
> deployments, use a managed Postgres service or a properly configured and
> secured Postgres instance.
-Install the chart with a persistent backend. From Polaris repo root:
+Install the dependencies in fixtures directory. From Polaris repo root:
Review Comment:
I think that is the current behavior of the doc and we have WARNING banner
right after `Persistent backend` already for this. Any preferred changes you
have?
##########
helm/polaris/README.md.gotmpl:
##########
@@ -198,27 +157,18 @@ The following tools are required to run the tests:
* [Chart Testing](https://github.com/helm/chart-testing)
Quick installation instructions for these tools:
-
```bash
helm plugin install https://github.com/helm-unittest/helm-unittest.git
brew install chart-testing
```
-The integration tests also require some fixtures to be deployed. The
`ci/fixtures` directory
-contains the required resources. To deploy them, run the following command:
+The integration tests also require some fixtures to be deployed. Follow the
above commands to setup required resources.
-```bash
-kubectl apply --namespace polaris -f helm/polaris/ci/fixtures/
-kubectl wait --namespace polaris --for=condition=ready pod
--selector=app.kubernetes.io/name=postgres --timeout=120s
-```
-
-The `helm/polaris/ci` contains a number of values files that will be used to
install the chart with
-different configurations.
+The `helm/polaris/ci` contains a number of values files that will be used to
install the chart with different configurations.
### Running the unit tests
-Helm unit tests do not require a Kubernetes cluster. To run the unit tests,
execute Helm Unit from
-the Polaris repo root:
+Helm unit tests do not require a Kubernetes cluster. To run the unit tests,
execute Helm Unit from the Polaris repo root:
Review Comment:
Yes. I tried to keep them become one line as oppose to multi-line (we
defined max characters per lines?) then format properly for those?
##########
helm/polaris/README.md.gotmpl:
##########
@@ -232,13 +182,11 @@ ct lint --charts helm/polaris
### Running the integration tests
-Integration tests require a Kubernetes cluster. See installation instructions
above for setting up
-a local cluster.
+Integration tests require a Kubernetes cluster. See installation instructions
above for setting upa local cluster.
Review Comment:
Fixed. Same reason as above. Change comment to one line instead of
multi-lines (we no max characters per line defined.)
##########
helm/polaris/templates/configmap.yaml:
##########
@@ -6,9 +6,9 @@
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
-
+
Review Comment:
Yes. Those were introduced with editor I am using which auto remove empty
space for the line. Let me revert it back.
##########
run.sh:
##########
@@ -53,9 +48,11 @@ sh ./kind-registry.sh
# Build and deploy the server image
echo "Building polaris image..."
./gradlew \
- :polaris-server:build \
+ :polaris-server:assemble \
:polaris-server:quarkusAppPartsBuild --rerun \
- $ECLIPSE_LINK_DEPS \
+ :polaris-admin:assemble \
+ :polaris-admin:quarkusAppPartsBuild --rerun \
+ -Dquarkus.container-image.tag=postgres-latest \
Review Comment:
Without `-Dquarkus.container-image.tag=postgres-latest`, it will introduce
breaking changes in other getting start examples as they are all refers to
`postgres-latest`. If this not preferred, we can remove this and update docs
that refs to this. Without using this, we will have `latest` and `<version>`
tags.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
