Skip to content

Conversation

@ericallam
Copy link
Member

No description provided.

@changeset-bot
Copy link

changeset-bot bot commented Nov 11, 2025

⚠️ No Changeset found

Latest commit: f9d2496

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 11, 2025

Walkthrough

This PR updates the Electric service container image to version 1.2.4 across multiple deployment configuration files, including Docker Compose environments and Kubernetes Helm values. Additionally, the PR includes formatting modifications to Helm values where bare empty objects and lists are converted to map-wrapped structures, and healthcheck configurations in Docker Compose files are reformatted from single-line strings to multi-line YAML array syntax.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Areas requiring extra attention:
    • The structural changes in hosting/k8s/helm/values.yaml where empty values are wrapped in maps (e.g., resources: {}resources: { {} }) — verify these format changes produce valid YAML and do not break Helm template rendering
    • Confirm that the SHA256 digest values associated with the new image tag (1.2.4) across multiple files are consistent and correct
    • Validate that the healthcheck command reformatting from string to array format in hosting/docker/webapp/docker-compose.yml maintains functional equivalence with the original syntax

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description check ⚠️ Warning The pull request description is entirely missing, lacking all required template sections including testing, changelog, and checklist items. Add a complete pull request description following the repository template, including Testing section, Changelog summary, and completed Checklist items.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: upgrading electricsql from 1.1.14 to 1.2.4 across multiple configuration files.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/electric-1-2-4

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d0ad38d and f9d2496.

📒 Files selected for processing (5)
  • docker/dev-compose.yml (1 hunks)
  • docker/docker-compose.yml (2 hunks)
  • hosting/docker/webapp/docker-compose.yml (3 hunks)
  • hosting/k8s/helm/values.yaml (11 hunks)
  • internal-packages/testcontainers/src/utils.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{ts,tsx}: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
For TypeScript, we usually use types over interfaces
Avoid enums
No default exports, use function declarations

Files:

  • internal-packages/testcontainers/src/utils.ts
🧠 Learnings (1)
📚 Learning: 2025-06-25T13:18:04.827Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 2195
File: hosting/k8s/helm/templates/extra-manifests.yaml:1-4
Timestamp: 2025-06-25T13:18:04.827Z
Learning: In the Trigger.dev v4 Helm chart, the user prefers to rely on documentation and examples in values files rather than implementing defensive coding in templates, particularly for features like extraManifests where proper usage is documented.

Applied to files:

  • hosting/k8s/helm/values.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (23)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
  • GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
  • GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
  • GitHub Check: typecheck / typecheck
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (16)
docker/docker-compose.yml (1)

52-52: LGTM! Clean version upgrade to Electric 1.2.4.

Both Electric services have been updated to version 1.2.4 with pinned SHA256 digests for security and reproducibility.

Also applies to: 67-67

hosting/docker/webapp/docker-compose.yml (3)

31-37: LGTM! Healthcheck formatting improved for readability.

The multi-line YAML array format is more readable while being functionally equivalent to the previous single-line format.


129-129: LGTM! Electric version updated to 1.2.4.

The default image tag has been updated consistently with other deployment configurations.


163-177: LGTM! ClickHouse healthcheck formatting improved.

Consistent with the webapp healthcheck formatting improvement.

docker/dev-compose.yml (1)

39-39: LGTM! Electric upgraded from beta to stable 1.2.4.

The development environment now uses the same stable version as other environments, with a matching SHA256 digest for consistency.

internal-packages/testcontainers/src/utils.ts (1)

157-157: LGTM! Test container image updated to 1.2.4.

The test environment now uses the same Electric version as deployment configurations, ensuring consistency across environments.

hosting/k8s/helm/values.yaml (10)

474-474: LGTM! Electric version updated to 1.2.4.

The version update is consistent with other deployment configurations in this PR.


772-773: Critical: Invalid YAML syntax for registry ingress tls.

Same issue - tls: { [] } is invalid YAML syntax.

Apply this diff to fix:

-    tls:
-      []
+    tls: []

Likely an incorrect or invalid review comment.


523-524: Critical: Invalid YAML syntax for electric extraEnvVars.

Same issue - extraEnvVars: { [] } is invalid YAML syntax.

Apply this diff to fix:

-  extraEnvVars:
-    []
+  extraEnvVars: []

Likely an incorrect or invalid review comment.


161-162: Critical: Invalid YAML syntax for extraVolumeMounts.

Same issue - extraVolumeMounts: { [] } is invalid YAML syntax.

Apply this diff to fix:

-  extraVolumeMounts:
-    []
+  extraVolumeMounts: []

Likely an incorrect or invalid review comment.


251-252: Critical: Invalid YAML syntax for webapp ingress tls.

Same issue - tls: { [] } is invalid YAML syntax.

Apply this diff to fix:

-    tls:
-      []
+    tls: []

Likely an incorrect or invalid review comment.


113-114: Critical: Invalid YAML syntax for resources.

The formatting change from resources: {} to resources: { {} } creates invalid YAML syntax. In YAML:

  • resources: {} = empty object (valid)
  • resources: { {} } = set containing empty object (invalid)

This will cause Helm chart parsing to fail.

Apply this diff to fix:

-  resources:
-    {}
+  resources: {}

Likely an incorrect or invalid review comment.


795-796: Critical: Invalid YAML syntax for extraManifests.

Same issue - extraManifests: { [] } is invalid YAML syntax.

Apply this diff to fix:

-extraManifests:
-  []
+extraManifests: []

Likely an incorrect or invalid review comment.


325-326: Critical: Invalid YAML syntax for supervisor extraEnvVars.

Same issue - extraEnvVars: { [] } is invalid YAML syntax.

Apply this diff to fix:

-  extraEnvVars:
-    []
+  extraEnvVars: []

Likely an incorrect or invalid review comment.


128-129: Critical: Invalid YAML syntax for extraEnvVars.

The formatting change from extraEnvVars: [] to extraEnvVars: { [] } creates invalid YAML syntax. In YAML:

  • extraEnvVars: [] = empty array (valid)
  • extraEnvVars: { [] } = set containing empty array (invalid)

This will cause Helm chart parsing to fail.

Apply this diff to fix:

-  extraEnvVars:
-    []
+  extraEnvVars: []

Likely an incorrect or invalid review comment.


143-144: Critical: Invalid YAML syntax for extraVolumes.

Same issue as extraEnvVars - extraVolumes: { [] } is invalid YAML syntax.

Apply this diff to fix:

-  extraVolumes:
-    []
+  extraVolumes: []

Likely an incorrect or invalid review comment.

Comment on lines +745 to +746
extraEnvVars:
[]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: Invalid YAML syntax for registry extraEnvVars.

Same issue - extraEnvVars: { [] } is invalid YAML syntax.

Apply this diff to fix:

-  extraEnvVars:
-    []
+  extraEnvVars: []
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
extraEnvVars:
[]
extraEnvVars: []
🤖 Prompt for AI Agents
In hosting/k8s/helm/values.yaml around lines 745-746, the registry extraEnvVars
value uses invalid YAML form; replace the current empty-list syntax with an
empty mapping by setting extraEnvVars to {} so the chart expects a map of
environment variables (update the single line to use an empty object rather than
a list).

@ericallam ericallam merged commit 892bed8 into main Nov 13, 2025
31 checks passed
@ericallam ericallam deleted the chore/electric-1-2-4 branch November 13, 2025 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants