Skip to content

Commit

Permalink
Merge pull request #188 from langchain-ai/brian/ls-2406-hybrid-params
Browse files Browse the repository at this point in the history
ci: add helm-unittest & test hybrid params
  • Loading branch information
bvs-langchain authored Nov 27, 2024
2 parents bec2b81 + fa408f8 commit 2a91e4a
Show file tree
Hide file tree
Showing 6 changed files with 62 additions and 3 deletions.
6 changes: 6 additions & 0 deletions .github/workflows/helm_checks.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,12 @@ jobs:
- name: Run chart-testing (lint)
run: ct lint --target-branch ${{ github.event.repository.default_branch }} --charts charts/${{ matrix.chart-name }}

- name: Install Helm Unittest
run: helm plugin install https://github.com/helm-unittest/helm-unittest.git --version 0.7.0

- name: Run chart unittest
run: helm unittest charts/${{ matrix.chart-name }}

- name: Create kind cluster
uses: helm/kind-action@v1.7.0

Expand Down
2 changes: 2 additions & 0 deletions charts/langsmith/.helmignore
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,5 @@
.idea/
*.tmproj
.vscode/
# Helm Unittest
tests/
2 changes: 2 additions & 0 deletions charts/langsmith/templates/_helpers.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,8 @@ Template containing common environment variables that are used by several servic
secretKeyRef:
name: {{ include "langsmith.redisSecretsName" . }}
key: connection_url
- name: CLICKHOUSE_HYBRID
value: {{ .Values.clickhouse.external.hybrid | quote }}
- name: CLICKHOUSE_DB
valueFrom:
secretKeyRef:
Expand Down
46 changes: 46 additions & 0 deletions charts/langsmith/tests/langsmith_hybrid_test.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
suite: test hybrid environment variables
values:
- ./values/blob-storage-defaults-enabled.yaml
templates:
- backend/clickhouse-migrations.yaml
- backend/deployment.yaml
- backend/postgres-migrations.yaml
- platform-backend/deployment.yaml
- queue/deployment.yaml
tests:
- it: should not have hybrid overrides set
asserts:
- contains:
path: spec.template.spec.containers[0].env
content:
name: CLICKHOUSE_HYBRID
value: "false"
- contains:
path: spec.template.spec.containers[0].env
content:
name: MIN_BLOB_STORAGE_SIZE_KB
value: "20"
- contains:
path: spec.template.spec.containers[0].env
content:
name: FF_CH_SEARCH_ENABLED
value: "true"
- it: should have hybrid overrides set
set:
clickhouse.external.hybrid: true
asserts:
- contains:
path: spec.template.spec.containers[0].env
content:
name: CLICKHOUSE_HYBRID
value: "true"
- contains:
path: spec.template.spec.containers[0].env
content:
name: MIN_BLOB_STORAGE_SIZE_KB
value: "0"
- contains:
path: spec.template.spec.containers[0].env
content:
name: FF_CH_SEARCH_ENABLED
value: "false"
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
config:
blobStorage:
enabled: true
6 changes: 3 additions & 3 deletions charts/langsmith/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -113,10 +113,10 @@ config:
engine: "S3"
# If you are using langsmith-managed-clickhouse, you may not want inputs to be stored in clickhouse for search.
# Set this as false to ensure that inputs/outputs/errors are not stored in clickhouse.
# hosting: "hybrid" overrides this to false.
# 'clickhouse.external.hybrid: true' overrides this to false.
chSearchEnabled: true
# Set this to change the threshold for payloads to be stored in blob storage
# hosting: "hybrid" overrides this to 0
# 'clickhouse.external.hybrid: true' overrides this to 0
minBlobStorageSizeKb: "20"
# If you are using workload identity, you may not need to store the S3 credentials in the secrets.
# Instead, you will need to add the workload identity annotation to the backend and queue service accounts.
Expand Down Expand Up @@ -396,7 +396,7 @@ clickhouse:
# If enabled, use the following values to connect to an external database. This will also disable the
# creation of a clickhouse stateful-set and service.
enabled: false
# -- Set to true if using managed ClickHouse
# -- Must be set to true if using managed ClickHouse
hybrid: false
host: ""
port: "8123"
Expand Down

0 comments on commit 2a91e4a

Please sign in to comment.