Copilot commented on code in PR #9511:
URL: https://github.com/apache/gravitino/pull/9511#discussion_r2638527349
##########
dev/charts/gravitino-lance-rest-server/templates/_helpers.tpl:
##########
@@ -0,0 +1,88 @@
+{{- /*
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ 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
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+ */}}
+
+{{/*
+Expand the name of the chart.
+*/}}
+{{- define "gravitino-lance-rest-server.name" -}}
+{{- default .Chart.Name .Values.nameOverride | trunc 63 | trimSuffix "-" }}
+{{- end }}
+
+{{/*
+Create a default fully qualified app name.
+We truncate at 63 chars because some Kubernetes name fields are limited to
this (by the DNS naming spec).
+If release name contains chart name it will be used as a full name.
+*/}}
+{{- define "gravitino-lance-rest-server.fullname" -}}
+{{- if .Values.fullnameOverride }}
+{{- .Values.fullnameOverride | trunc 63 | trimSuffix "-" }}
+{{- else }}
+{{- $name := default .Chart.Name .Values.nameOverride }}
+{{- if contains $name .Release.Name }}
+{{- .Release.Name | trunc 63 | trimSuffix "-" }}
+{{- else }}
+{{- printf "%s-%s" .Release.Name $name | trunc 63 | trimSuffix "-" }}
+{{- end }}
+{{- end }}
+{{- end }}
+
+{{/*
+Create chart name and version as used by the chart label.
+*/}}
+{{- define "gravitino-lance-rest-server.chart" -}}
+{{- printf "%s-%s" .Chart.Name .Chart.Version | replace "+" "_" | trunc 63 |
trimSuffix "-" }}
+{{- end }}
+
+{{/*
+Common labels
+*/}}
+{{- define "gravitino-lance-rest-server.labels" -}}
+helm.sh/chart: {{ include "gravitino-lance-rest-server.chart" . }}
+{{ include "gravitino-lance-rest-server.selectorLabels" . }}
+{{- if .Chart.AppVersion }}
+app.kubernetes.io/version: {{ .Chart.AppVersion | quote }}
+{{- end }}
+app.kubernetes.io/managed-by: {{ .Release.Service }}
+{{- end }}
+
+{{/*
+Selector labels
+*/}}
+{{- define "gravitino-lance-rest-server.selectorLabels" -}}
+app.kubernetes.io/name: {{ include "gravitino-lance-rest-server.name" . }}
+app.kubernetes.io/instance: {{ .Release.Name }}
+{{- end }}
+
+{{/*
+Create the name of the service account to use
+*/}}
+{{- define "gravitino-lance-rest-server.serviceAccountName" -}}
+{{- if .Values.serviceAccount.create }}
+{{- default (include "gravitino-lance-rest-server.fullname" .)
.Values.serviceAccount.name }}
+{{- else }}
+{{- default "default" .Values.serviceAccount.name }}
+{{- end }}
+{{- end }}
+
+{{/*
+Define the gravitino.namespace
+*/}}
+{{- define "gravitino-lance-rest-server.namespace" -}}
+ {{- .Release.Namespace -}}
Review Comment:
The namespace helper template has inconsistent indentation with a leading
tab. This could cause issues with YAML rendering. The helper should return the
namespace value without extra whitespace.
```suggestion
{{- .Release.Namespace -}}
```
##########
docs/lance-rest-server-chart.md:
##########
@@ -0,0 +1,99 @@
+---
+title: "Install Lance Rest Server on Kubernetes"
+slug: /lance-rest-server-chart
+keyword:
+ - Lance REST Server Helm Chart
+license: "This software is licensed under the Apache License version 2."
+---
+
+# Install Lance Rest Server on Kubernetes
+
+This Helm chart deploys Apache Gravitino Lance REST Server on Kubernetes with
customizable configurations.
+
+## Prerequisites
+
+- Kubernetes 1.29+
+- Helm 3+
+
+## Update Chart Dependency
+
+The Gravitino Lance REST Server Helm chart has not yet been officially
released.
+To proceed, please clone the repository, navigate to the chart directory
[charts](../dev/charts), and execute the Helm dependency update command.
+
+```console
+helm dependency update [CHART]
+```
+
+## View Chart values
+
+You can customize values.yaml parameters to override chart default settings.
Additionally, Gravitino Lance REST Server configurations in
[gravitino-lance-rest-server.conf](../dev/charts/gravitino-lance-rest-server/resources/gravitino-lance-rest-server.conf)
can be modified through Helm values.yaml.
+
+To display the default values of the chart, run:
+
+```console
+helm show values [CHART]
+```
+
+## Install Helm Chart
+
+```console
+helm install [RELEASE_NAME] [CHART] [flags]
+```
+
+### Deploy with Default Configuration
+
+Run the following command to deploy Gravitino Lance REST Server using the
default settings, specify container image versions using --set image.tag=x.y.z
(replace x, y, z with the expected version numbers):
+
+```console
+helm upgrade --install gravitino ./gravitino-lance-rest-server \
+ -n gravitino \
+ --create-namespace \
+ --set image.tag=<x.y.z> \
Review Comment:
The placeholder text in the command example uses inconsistent notation. The
angle brackets should be used consistently without additional x, y, z
variables. Consider using a more explicit version number like "1.1.0" or just
"<version>" for clarity.
```suggestion
Run the following command to deploy Gravitino Lance REST Server using the
default settings, specifying the container image version using `--set
image.tag=<version>` (replace `<version>` with the desired image tag):
```console
helm upgrade --install gravitino ./gravitino-lance-rest-server \
-n gravitino \
--create-namespace \
--set image.tag=<version> \
```
##########
dev/charts/gravitino-lance-rest-server/values.yaml:
##########
@@ -0,0 +1,183 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# 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
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+# Default values for gravitino-lance-rest-server.
+# This is a YAML-formatted file.
+# Declare variables to be passed into your templates.
+
+replicaCount: 1
+
+image:
+ repository: apache/gravitino-lance-rest
+ pullPolicy: IfNotPresent
+ # Overrides the image tag whose default is the chart appVersion.
+ tag: "1.1.0"
+
+imagePullSecrets: []
+nameOverride: ""
+fullnameOverride: ""
+
+annotations: {}
+
+## Lance REST server configuration
+lanceRest:
+ shutdownTimeout: 3000
+ host: 0.0.0.0
+ httpPort: 9101
+ minThreads: 24
+ maxThreads: 200
+ stopTimeout: 30000
+ idleTimeout: 30000
+ threadPoolWorkQueueSize: 100
+ requestHeaderSize: 131072
+ responseHeaderSize: 131072
+ # Lance namespace backend settings
+ namespaceBackend: gravitino
+ gravitinoUri: http://192.168.194.200:8090
+ gravitinoMetalake: "lrs_test"
+# Rest backend configs.
+additionalConfigItems: {}
+
+## Log4j2 configuration items
+log4j2Properties:
+ status: debug
Review Comment:
The log4j2Properties status is set to "debug" which is very verbose for
production use. This should default to "warn" or "info" to avoid excessive
logging overhead. Debug level logging should be explicitly enabled only when
needed for troubleshooting.
```suggestion
status: info
```
##########
docs/lance-rest-server-chart.md:
##########
@@ -0,0 +1,99 @@
+---
+title: "Install Lance Rest Server on Kubernetes"
+slug: /lance-rest-server-chart
+keyword:
+ - Lance REST Server Helm Chart
+license: "This software is licensed under the Apache License version 2."
+---
+
+# Install Lance Rest Server on Kubernetes
Review Comment:
The documentation title and several references use "Rest" instead of "REST".
REST is an acronym (REpresentational State Transfer) and should be capitalized
consistently throughout the documentation.
```suggestion
title: "Install Lance REST Server on Kubernetes"
slug: /lance-rest-server-chart
keyword:
- Lance REST Server Helm Chart
license: "This software is licensed under the Apache License version 2."
---
# Install Lance REST Server on Kubernetes
```
##########
docs/lance-rest-server-chart.md:
##########
@@ -0,0 +1,99 @@
+---
+title: "Install Lance Rest Server on Kubernetes"
+slug: /lance-rest-server-chart
+keyword:
+ - Lance REST Server Helm Chart
+license: "This software is licensed under the Apache License version 2."
+---
+
+# Install Lance Rest Server on Kubernetes
Review Comment:
The documentation title and several references use "Rest" instead of "REST".
REST is an acronym (REpresentational State Transfer) and should be capitalized
consistently throughout the documentation.
```suggestion
title: "Install Lance REST Server on Kubernetes"
slug: /lance-rest-server-chart
keyword:
- Lance REST Server Helm Chart
license: "This software is licensed under the Apache License version 2."
---
# Install Lance REST Server on Kubernetes
```
##########
docs/lance-rest-server-chart.md:
##########
@@ -0,0 +1,99 @@
+---
+title: "Install Lance Rest Server on Kubernetes"
+slug: /lance-rest-server-chart
+keyword:
+ - Lance REST Server Helm Chart
+license: "This software is licensed under the Apache License version 2."
+---
+
+# Install Lance Rest Server on Kubernetes
+
+This Helm chart deploys Apache Gravitino Lance REST Server on Kubernetes with
customizable configurations.
+
+## Prerequisites
+
+- Kubernetes 1.29+
+- Helm 3+
+
+## Update Chart Dependency
+
+The Gravitino Lance REST Server Helm chart has not yet been officially
released.
+To proceed, please clone the repository, navigate to the chart directory
[charts](../dev/charts), and execute the Helm dependency update command.
+
+```console
+helm dependency update [CHART]
+```
+
+## View Chart values
+
+You can customize values.yaml parameters to override chart default settings.
Additionally, Gravitino Lance REST Server configurations in
[gravitino-lance-rest-server.conf](../dev/charts/gravitino-lance-rest-server/resources/gravitino-lance-rest-server.conf)
can be modified through Helm values.yaml.
+
+To display the default values of the chart, run:
+
+```console
+helm show values [CHART]
+```
+
+## Install Helm Chart
+
+```console
+helm install [RELEASE_NAME] [CHART] [flags]
+```
+
+### Deploy with Default Configuration
+
+Run the following command to deploy Gravitino Lance REST Server using the
default settings, specify container image versions using --set image.tag=x.y.z
(replace x, y, z with the expected version numbers):
+
+```console
+helm upgrade --install gravitino ./gravitino-lance-rest-server \
+ -n gravitino \
+ --create-namespace \
+ --set image.tag=<x.y.z> \
+ --set replicas=2 \
+ --set resources.requests.memory="4Gi" \
+ --set resources.requests.cpu="2"
+```
+
+### Deploy with Custom Configuration
+
+To customize the deployment, use the --set flag to override specific values:
+
+```console
+helm upgrade --install gravitino ./gravitino-lance-rest-server
+ -n gravitino \
+ --create-namespace \
+ --set key1=val1,key2=val2,...
+```
+Alternatively, you can provide a custom values.yaml file:
+
+```console
+helm upgrade --install gravitino ./gravitino-lance-rest-server
Review Comment:
The command is missing a backslash for line continuation. Without it, the
command will fail as the shell will interpret line 70 as the complete command.
##########
dev/charts/gravitino-lance-rest-server/values.yaml:
##########
@@ -0,0 +1,183 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# 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
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+# Default values for gravitino-lance-rest-server.
+# This is a YAML-formatted file.
+# Declare variables to be passed into your templates.
+
+replicaCount: 1
+
+image:
+ repository: apache/gravitino-lance-rest
+ pullPolicy: IfNotPresent
+ # Overrides the image tag whose default is the chart appVersion.
+ tag: "1.1.0"
+
+imagePullSecrets: []
+nameOverride: ""
+fullnameOverride: ""
+
+annotations: {}
+
+## Lance REST server configuration
+lanceRest:
+ shutdownTimeout: 3000
+ host: 0.0.0.0
+ httpPort: 9101
+ minThreads: 24
+ maxThreads: 200
+ stopTimeout: 30000
+ idleTimeout: 30000
+ threadPoolWorkQueueSize: 100
+ requestHeaderSize: 131072
+ responseHeaderSize: 131072
+ # Lance namespace backend settings
+ namespaceBackend: gravitino
+ gravitinoUri: http://192.168.194.200:8090
Review Comment:
The default value for gravitinoUri appears to be a hardcoded internal IP
address (192.168.194.200). This should be changed to a placeholder value like
"http://gravitino-server:8090"; or "http://localhost:8090"; to avoid confusion
and prevent users from accidentally using this development/test IP address in
production.
```suggestion
gravitinoUri: http://gravitino-server:8090
```
##########
docs/lance-rest-server-chart.md:
##########
@@ -0,0 +1,99 @@
+---
+title: "Install Lance Rest Server on Kubernetes"
+slug: /lance-rest-server-chart
+keyword:
+ - Lance REST Server Helm Chart
+license: "This software is licensed under the Apache License version 2."
+---
+
+# Install Lance Rest Server on Kubernetes
+
+This Helm chart deploys Apache Gravitino Lance REST Server on Kubernetes with
customizable configurations.
+
+## Prerequisites
+
+- Kubernetes 1.29+
+- Helm 3+
+
+## Update Chart Dependency
+
+The Gravitino Lance REST Server Helm chart has not yet been officially
released.
+To proceed, please clone the repository, navigate to the chart directory
[charts](../dev/charts), and execute the Helm dependency update command.
+
+```console
+helm dependency update [CHART]
+```
+
+## View Chart values
+
+You can customize values.yaml parameters to override chart default settings.
Additionally, Gravitino Lance REST Server configurations in
[gravitino-lance-rest-server.conf](../dev/charts/gravitino-lance-rest-server/resources/gravitino-lance-rest-server.conf)
can be modified through Helm values.yaml.
+
+To display the default values of the chart, run:
+
+```console
+helm show values [CHART]
+```
+
+## Install Helm Chart
+
+```console
+helm install [RELEASE_NAME] [CHART] [flags]
+```
+
+### Deploy with Default Configuration
+
+Run the following command to deploy Gravitino Lance REST Server using the
default settings, specify container image versions using --set image.tag=x.y.z
(replace x, y, z with the expected version numbers):
+
+```console
+helm upgrade --install gravitino ./gravitino-lance-rest-server \
+ -n gravitino \
+ --create-namespace \
+ --set image.tag=<x.y.z> \
+ --set replicas=2 \
+ --set resources.requests.memory="4Gi" \
+ --set resources.requests.cpu="2"
+```
+
+### Deploy with Custom Configuration
+
+To customize the deployment, use the --set flag to override specific values:
+
+```console
+helm upgrade --install gravitino ./gravitino-lance-rest-server
Review Comment:
The command is missing a backslash for line continuation. Without it, the
command will fail as the shell will interpret line 62 as the complete command.
##########
dev/charts/gravitino-lance-rest-server/values.yaml:
##########
@@ -0,0 +1,183 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# 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
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+# Default values for gravitino-lance-rest-server.
+# This is a YAML-formatted file.
+# Declare variables to be passed into your templates.
+
+replicaCount: 1
+
+image:
+ repository: apache/gravitino-lance-rest
+ pullPolicy: IfNotPresent
+ # Overrides the image tag whose default is the chart appVersion.
+ tag: "1.1.0"
+
+imagePullSecrets: []
+nameOverride: ""
+fullnameOverride: ""
+
+annotations: {}
+
+## Lance REST server configuration
+lanceRest:
+ shutdownTimeout: 3000
+ host: 0.0.0.0
+ httpPort: 9101
+ minThreads: 24
+ maxThreads: 200
+ stopTimeout: 30000
+ idleTimeout: 30000
+ threadPoolWorkQueueSize: 100
+ requestHeaderSize: 131072
+ responseHeaderSize: 131072
+ # Lance namespace backend settings
+ namespaceBackend: gravitino
+ gravitinoUri: http://192.168.194.200:8090
+ gravitinoMetalake: "lrs_test"
+# Rest backend configs.
+additionalConfigItems: {}
+
+## Log4j2 configuration items
+log4j2Properties:
+ status: debug
+additionalLog4j2Properties:
+ appender.console.type: Console
+ appender.console.name: consoleLogger
+ appender.console.layout.type: PatternLayout
+ appender.console.layout.pattern: "%d{yyyy-MM-dd HH:mm:ss} %-5p [%t] %c{1}:%L
- %m%n"
+ rootLogger.appenderRef.console.ref: consoleLogger
+
+initScript: |
+ echo "Override config."
+ cp /tmp/conf/* ${GRAVITINO_HOME}/conf
+ echo "Start the Gravitino Lance Rest Server"
+ /bin/bash ${GRAVITINO_HOME}/bin/gravitino-lance-rest-server.sh run
+
+## Optional volumes for logs
+volumes:
+ - name: gravitino-rest-catalog-server-log
+ emptyDir: {}
+
+volumeMounts:
+ - name: gravitino-rest-catalog-server-log
+ mountPath: /root/gravitino-lance-rest-server/logs
+
+env:
+ - name: GRAVITINO_HOME
+ value: /root/gravitino-lance-rest-server
+ - name: GRAVITINO_MEM
+ value: "-Xms1024m -Xmx1024m -XX:MaxMetaspaceSize=512m"
+
+envFrom: []
+
+serviceAccount:
+ # Specifies whether a service account should be created
+ create: false
+ # Automatically mount a ServiceAccount's API credentials
+ automount: true
+ # Annotations to add to the service account
+ annotations: {}
+ # The name of the service account to use.
+ # If not set and create is true, a name is generated using the fullname
template
+ name: ""
+
+podAnnotations: {}
+podLabels: {}
+
+podSecurityContext: {}
+ # fsGroup: 2000
+
+securityContext: {}
+ # capabilities:
+ # drop:
+ # - ALL
+ # readOnlyRootFilesystem: true
+ # runAsNonRoot: true
+ # runAsUser: 1000
+
+service:
+ name: gravitino-lance-rest-server
+ type: ClusterIP
+ port: 9101
+ targetPort: 9101
+ annotations: {}
+ labels: {}
+ portName: http
+ nodePort: ""
+
+livenessProbe:
+ httpGet:
+ path: /lance/v1/namespace/$/list
+ port: http
+ initialDelaySeconds: 60
+ timeoutSeconds: 10
+ periodSeconds: 30
+ failureThreshold: 5
+
+readinessProbe:
+ httpGet:
+ path: /lance/v1/namespace/$/list
Review Comment:
The health check endpoint path "/lance/v1/namespace/$/list" uses a literal
"$" character which needs to be URL-encoded as "%24" when used in HTTP
requests. While this may work in some contexts, the test pod correctly uses the
encoded version. Consider documenting this requirement or ensuring the probes
handle the encoding correctly.
##########
dev/charts/gravitino-lance-rest-server/values.yaml:
##########
@@ -0,0 +1,183 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# 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
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+# Default values for gravitino-lance-rest-server.
+# This is a YAML-formatted file.
+# Declare variables to be passed into your templates.
+
+replicaCount: 1
+
+image:
+ repository: apache/gravitino-lance-rest
+ pullPolicy: IfNotPresent
+ # Overrides the image tag whose default is the chart appVersion.
+ tag: "1.1.0"
+
+imagePullSecrets: []
+nameOverride: ""
+fullnameOverride: ""
+
+annotations: {}
+
+## Lance REST server configuration
+lanceRest:
+ shutdownTimeout: 3000
+ host: 0.0.0.0
+ httpPort: 9101
+ minThreads: 24
+ maxThreads: 200
+ stopTimeout: 30000
+ idleTimeout: 30000
+ threadPoolWorkQueueSize: 100
+ requestHeaderSize: 131072
+ responseHeaderSize: 131072
+ # Lance namespace backend settings
+ namespaceBackend: gravitino
+ gravitinoUri: http://192.168.194.200:8090
+ gravitinoMetalake: "lrs_test"
Review Comment:
The default metalake name "lrs_test" suggests this is a test/development
value. Consider using a more generic placeholder like "metalake" or documenting
that this must be changed for production use. The default should either be
empty (requiring explicit configuration) or use a more production-appropriate
name.
```suggestion
# Name of the target metalake; must be set for your environment.
gravitinoMetalake: ""
```
##########
dev/charts/gravitino-lance-rest-server/templates/tests/test-connection.yaml:
##########
@@ -0,0 +1,64 @@
+{{- /*
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ 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
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+ */}}
+
+apiVersion: v1
+kind: Pod
+metadata:
+ name: {{ include "gravitino-lance-rest-server.fullname" . }}-test-connection
+ namespace: {{ include "gravitino-lance-rest-server.namespace" . }}
+ annotations:
+ helm.sh/hook: test
+ helm.sh/hook-delete-policy: hook-succeeded
+spec:
+ containers:
+ - name: test-connection
+ image: curlimages/curl:latest
+ command:
+ - /bin/sh
+ - -c
+ - |
+ max_attempts=30
+ attempt=0
+ success=false
+
+ while [ $attempt -lt $max_attempts ]; do
+ http_code=$(curl -s -o /tmp/body -w "%{http_code}"
"http://${SERVICE_NAME}:${SERVICE_PORT}${TEST_PATH}";)
+ if [ "$http_code" = "200" ] && grep -qi "namespace" /tmp/body; then
+ success=true
+ break
+ else
+ echo "Attempt $((attempt + 1)) failed with code
${http_code:-n/a}" && cat /tmp/body || true
+ sleep 1
Review Comment:
The test connection script performs retries but only waits 1 second between
attempts with a maximum of 30 attempts. Given that the readiness probe has an
initialDelaySeconds of 30 and the liveness probe has 60, the test might fail
prematurely before the service is actually ready. Consider increasing the sleep
interval or the maximum number of attempts, or aligning with the probe timings
(e.g., wait 2-3 seconds between attempts).
##########
dev/charts/gravitino-lance-rest-server/values.yaml:
##########
@@ -0,0 +1,183 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# 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
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+# Default values for gravitino-lance-rest-server.
+# This is a YAML-formatted file.
+# Declare variables to be passed into your templates.
+
+replicaCount: 1
+
+image:
+ repository: apache/gravitino-lance-rest
+ pullPolicy: IfNotPresent
+ # Overrides the image tag whose default is the chart appVersion.
+ tag: "1.1.0"
+
+imagePullSecrets: []
+nameOverride: ""
+fullnameOverride: ""
+
+annotations: {}
+
+## Lance REST server configuration
+lanceRest:
+ shutdownTimeout: 3000
+ host: 0.0.0.0
+ httpPort: 9101
+ minThreads: 24
+ maxThreads: 200
+ stopTimeout: 30000
+ idleTimeout: 30000
+ threadPoolWorkQueueSize: 100
+ requestHeaderSize: 131072
+ responseHeaderSize: 131072
+ # Lance namespace backend settings
+ namespaceBackend: gravitino
+ gravitinoUri: http://192.168.194.200:8090
+ gravitinoMetalake: "lrs_test"
+# Rest backend configs.
+additionalConfigItems: {}
+
+## Log4j2 configuration items
+log4j2Properties:
+ status: debug
+additionalLog4j2Properties:
+ appender.console.type: Console
+ appender.console.name: consoleLogger
+ appender.console.layout.type: PatternLayout
+ appender.console.layout.pattern: "%d{yyyy-MM-dd HH:mm:ss} %-5p [%t] %c{1}:%L
- %m%n"
+ rootLogger.appenderRef.console.ref: consoleLogger
+
+initScript: |
+ echo "Override config."
+ cp /tmp/conf/* ${GRAVITINO_HOME}/conf
+ echo "Start the Gravitino Lance Rest Server"
+ /bin/bash ${GRAVITINO_HOME}/bin/gravitino-lance-rest-server.sh run
+
+## Optional volumes for logs
+volumes:
+ - name: gravitino-rest-catalog-server-log
+ emptyDir: {}
+
+volumeMounts:
+ - name: gravitino-rest-catalog-server-log
+ mountPath: /root/gravitino-lance-rest-server/logs
+
+env:
+ - name: GRAVITINO_HOME
+ value: /root/gravitino-lance-rest-server
+ - name: GRAVITINO_MEM
+ value: "-Xms1024m -Xmx1024m -XX:MaxMetaspaceSize=512m"
+
+envFrom: []
+
+serviceAccount:
+ # Specifies whether a service account should be created
+ create: false
+ # Automatically mount a ServiceAccount's API credentials
+ automount: true
+ # Annotations to add to the service account
+ annotations: {}
+ # The name of the service account to use.
+ # If not set and create is true, a name is generated using the fullname
template
+ name: ""
+
+podAnnotations: {}
+podLabels: {}
+
+podSecurityContext: {}
+ # fsGroup: 2000
+
+securityContext: {}
+ # capabilities:
+ # drop:
+ # - ALL
+ # readOnlyRootFilesystem: true
+ # runAsNonRoot: true
+ # runAsUser: 1000
+
+service:
+ name: gravitino-lance-rest-server
+ type: ClusterIP
+ port: 9101
+ targetPort: 9101
+ annotations: {}
+ labels: {}
+ portName: http
+ nodePort: ""
+
+livenessProbe:
+ httpGet:
+ path: /lance/v1/namespace/$/list
Review Comment:
The health check endpoint path "/lance/v1/namespace/$/list" uses a literal
"$" character which needs to be URL-encoded as "%24" when used in HTTP
requests. While this may work in some contexts, the test pod correctly uses the
encoded version. Consider documenting this requirement or ensuring the probes
handle the encoding correctly.
##########
dev/charts/gravitino-lance-rest-server/templates/deployment.yaml:
##########
@@ -0,0 +1,101 @@
+{{- /*
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ 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
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+ */}}
+
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+ name: {{ include "gravitino-lance-rest-server.fullname" . }}
+ labels:
+ {{- include "gravitino-lance-rest-server.labels" . | nindent 4 }}
+ annotations:
+ {{- toYaml .Values.annotations | nindent 4 }}
+spec:
+ replicas: {{ .Values.replicaCount }}
+ selector:
+ matchLabels:
+ {{- include "gravitino-lance-rest-server.selectorLabels" . | nindent 6 }}
+ template:
+ metadata:
+ annotations:
+ checksum/config: {{ include (print $.Template.BasePath
"/configmap.yaml") . | sha256sum }}
+ {{- with .Values.podAnnotations }}
+ {{- toYaml . | nindent 8 }}
+ {{- end }}
+ labels:
+ {{- include "gravitino-lance-rest-server.labels" . | nindent 8 }}
+ {{- with .Values.podLabels }}
+ {{- toYaml . | nindent 8 }}
+ {{- end }}
+ spec:
+ {{- with .Values.imagePullSecrets }}
+ imagePullSecrets:
+ {{- toYaml . | nindent 8 }}
+ {{- end }}
+ serviceAccountName: {{ include
"gravitino-lance-rest-server.serviceAccountName" . }}
+ securityContext:
+ {{- toYaml .Values.podSecurityContext | nindent 8 }}
+ containers:
+ - name: {{ .Chart.Name }}
+ securityContext:
+ {{- toYaml .Values.securityContext | nindent 12 }}
+ image: "{{ .Values.image.repository }}:{{ .Values.image.tag |
default .Chart.AppVersion }}"
+ imagePullPolicy: {{ .Values.image.pullPolicy }}
+ command:
+ - "/bin/bash"
+ - "/tmp/conf/init.sh"
+ # Environment variables
+ {{- if .Values.env }}
+ env:
+ {{- toYaml .Values.env | nindent 12 }}
+ {{- end }}
Review Comment:
The deployment template only includes the env section when Values.env is
set, but does not include envFrom even though it's defined in values.yaml. This
means users cannot use envFrom configuration (for secrets or configmaps) even
though it's documented in the values file. The template should also
conditionally include envFrom.
```suggestion
{{- end }}
{{- with .Values.envFrom }}
envFrom:
{{- toYaml . | nindent 12 }}
{{- end }}
```
##########
dev/charts/gravitino-lance-rest-server/templates/deployment.yaml:
##########
@@ -0,0 +1,101 @@
+{{- /*
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ 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
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+ */}}
+
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+ name: {{ include "gravitino-lance-rest-server.fullname" . }}
+ labels:
+ {{- include "gravitino-lance-rest-server.labels" . | nindent 4 }}
+ annotations:
+ {{- toYaml .Values.annotations | nindent 4 }}
+spec:
+ replicas: {{ .Values.replicaCount }}
+ selector:
+ matchLabels:
+ {{- include "gravitino-lance-rest-server.selectorLabels" . | nindent 6 }}
+ template:
+ metadata:
+ annotations:
+ checksum/config: {{ include (print $.Template.BasePath
"/configmap.yaml") . | sha256sum }}
+ {{- with .Values.podAnnotations }}
+ {{- toYaml . | nindent 8 }}
+ {{- end }}
+ labels:
+ {{- include "gravitino-lance-rest-server.labels" . | nindent 8 }}
+ {{- with .Values.podLabels }}
+ {{- toYaml . | nindent 8 }}
+ {{- end }}
+ spec:
+ {{- with .Values.imagePullSecrets }}
+ imagePullSecrets:
+ {{- toYaml . | nindent 8 }}
+ {{- end }}
+ serviceAccountName: {{ include
"gravitino-lance-rest-server.serviceAccountName" . }}
+ securityContext:
+ {{- toYaml .Values.podSecurityContext | nindent 8 }}
+ containers:
+ - name: {{ .Chart.Name }}
+ securityContext:
+ {{- toYaml .Values.securityContext | nindent 12 }}
+ image: "{{ .Values.image.repository }}:{{ .Values.image.tag |
default .Chart.AppVersion }}"
+ imagePullPolicy: {{ .Values.image.pullPolicy }}
+ command:
+ - "/bin/bash"
+ - "/tmp/conf/init.sh"
+ # Environment variables
+ {{- if .Values.env }}
+ env:
+ {{- toYaml .Values.env | nindent 12 }}
+ {{- end }}
+ ports:
+ - name: http
+ containerPort: {{ .Values.service.port }}
Review Comment:
The containerPort is set to the service port value (Values.service.port),
but it should be set to Values.lanceRest.httpPort which is the actual port the
application listens on (9101). While these may have the same default value,
they represent different concepts - the application's listening port vs the
service's exposed port. If someone changes the service port without changing
lanceRest.httpPort, the container won't be reachable.
```suggestion
containerPort: {{ .Values.lanceRest.httpPort }}
```
##########
dev/charts/gravitino-lance-rest-server/resources/gravitino-lance-rest-server.conf:
##########
@@ -0,0 +1,50 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# 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
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+# THE CONFIGURATION FOR Lance REST SERVER
+gravitino.lance-rest.shutdown.timeout = {{ .Values.lanceRest.shutdownTimeout |
default 3000 }}
+
+# THE CONFIGURATION FOR Lance REST WEB SERVER
+# The host name of the built-in web server
+gravitino.lance-rest.host = {{ .Values.lanceRest.host | default "0.0.0.0" }}
+gravitino.lance-rest.httpPort = {{ .Values.lanceRest.httpPort | default 9101 }}
+gravitino.lance-rest.minThreads = {{ .Values.lanceRest.minThreads | default 24
}}
+gravitino.lance-rest.maxThreads = {{ .Values.lanceRest.maxThreads | default
200 }}
+gravitino.lance-rest.stopTimeout = {{ .Values.lanceRest.stopTimeout | default
30000 }}
+gravitino.lance-rest.idleTimeout = {{ .Values.lanceRest.idleTimeout | default
30000 }}
+gravitino.lance-rest.threadPoolWorkQueueSize = {{
.Values.lanceRest.threadPoolWorkQueueSize | default 100 }}
+gravitino.lance-rest.requestHeaderSize = {{
.Values.lanceRest.requestHeaderSize | default 131072 }}
+gravitino.lance-rest.responseHeaderSize = {{
.Values.lanceRest.responseHeaderSize | default 131072 }}
+
+# THE CONFIGURATION FOR Lance namespace backend
+# The backend Lance namespace for Lance REST service, it's recommended to use
Gravitino
+gravitino.lance-rest.namespace-backend = {{ .Values.lanceRest.namespaceBackend
| default "gravitino" }}
+# The uri of the Lance REST service gravitino namespace backend
+gravitino.lance-rest.gravitino-uri = {{ .Values.lanceRest.gravitinoUri |
default "http://localhost:8090"; }}
+# The metalake name used for Lance REST service gravitino namespace backend,
please create the metalake before using it, and configure the metalake name
here.
+{{- if .Values.lanceRest.gravitinoMetalake }}
+gravitino.lance-rest.gravitino-metalake = {{
.Values.lanceRest.gravitinoMetalake }}
+{{- else }}
+# gravitino.lance-rest.gravitino-metalake = metalake
Review Comment:
The configuration template conditionally includes the gravitino-metalake
setting only if it's provided, otherwise it comments it out. However, based on
the documentation, the metalake is required for the Lance REST server to
function. Consider either making this a required value with validation, or
providing a clearer error message when it's not set. Users may deploy the chart
successfully but have a non-functional service if this crucial configuration is
missing.
```suggestion
# WARNING: lanceRest.gravitinoMetalake is not set. Lance REST server
requires an existing Gravitino metalake.
# Please set .Values.lanceRest.gravitinoMetalake to the name of a
pre-created metalake before deploying this chart.
gravitino.lance-rest.gravitino-metalake = metalake
```
--
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]
