Skip to content

Conversation

@xav-db
Copy link
Member

@xav-db xav-db commented Nov 15, 2025

Description

implementing docker build reading env vars

Related Issues

Closes #

Checklist when merging to main

  • No compiler warnings (if applicable)
  • Code is formatted with rustfmt
  • No useless or dead code (if applicable)
  • Code is easy to understand
  • Doc comments are used for all functions, enums, structs, and fields (where appropriate)
  • All tests pass
  • Performance has not regressed (assuming change was not to fix a bug)
  • Version number has been updated in helix-cli/Cargo.toml and helixdb/Cargo.toml

Additional Notes

Greptile Overview

Greptile Summary

This PR enables Docker containers to access embedding provider API keys by loading environment variables from .env files and the shell environment. The changes allow OPENAI_API_KEY and GEMINI_API_KEY to be passed through to containers, which is necessary for the embedding functionality in helix-db/src/helix_gateway/embedding_providers/mod.rs to work properly.

Key Changes:

  • Added dotenvy::dotenv() call in environment_variables() to load .env file
  • Added conditional environment variable passing for OPENAI_API_KEY and GEMINI_API_KEY
  • Refactored generate_docker_compose() to dynamically generate environment section from all variables
  • Removed Docker daemon auto-start logic (~112 lines removed)

Notes:

  • The .env file loading happens every time environment_variables() is called, which could be optimized
  • API keys are securely passed through Docker Compose environment variables (not logged to stdout)
  • The removed Docker auto-start functionality appears to be intentional cleanup

Important Files Changed

File Analysis

Filename Score Overview
helix-cli/src/docker.rs 4/5 Added environment variable loading from .env and shell for API keys (OPENAI_API_KEY, GEMINI_API_KEY) to support embedding providers, removed Docker daemon auto-start logic

Sequence Diagram

sequenceDiagram
    participant User as User
    participant CLI as Helix CLI
    participant DM as DockerManager
    participant Env as Environment/.env
    participant Docker as Docker Compose
    participant Container as Helix Container
    participant EP as Embedding Provider

    User->>CLI: helix start/build
    CLI->>DM: generate_docker_compose()
    DM->>DM: environment_variables(instance_name)
    DM->>Env: dotenvy::dotenv()
    Env-->>DM: Load .env file (silent fail)
    DM->>Env: std::env::var("OPENAI_API_KEY")
    Env-->>DM: Return key if exists
    DM->>Env: std::env::var("GEMINI_API_KEY")
    Env-->>DM: Return key if exists
    DM->>DM: Build env_vars vec (HELIX_PORT, HELIX_DATA_DIR, etc.)
    DM->>DM: Format environment section
    DM-->>CLI: docker-compose.yml content
    CLI->>Docker: docker compose up
    Docker->>Container: Start with env vars
    Container->>EP: API request with OPENAI_API_KEY/GEMINI_API_KEY
    EP-->>Container: Embedding response
Loading

@xav-db xav-db marked this pull request as ready for review November 15, 2025 00:40
@xav-db xav-db merged commit cc1b8f9 into dev Nov 15, 2025
19 checks passed
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

1 file reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

pub(crate) fn environment_variables(&self, instance_name: &str) -> Vec<String> {
vec![
// Load .env file (silently ignore if it doesn't exist)
let _ = dotenvy::dotenv();
Copy link
Contributor

Choose a reason for hiding this comment

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

style: dotenvy::dotenv() is called every time environment_variables() is invoked. Move this to a once-per-process initialization (e.g., in main() or using OnceLock) to avoid redundant .env file reads on every docker-compose generation or container operation

Suggested change
let _ = dotenvy::dotenv();
// .env loading should happen once at startup, not here
Prompt To Fix With AI
This is a comment left during a code review.
Path: helix-cli/src/docker.rs
Line: 50:50

Comment:
**style:** `dotenvy::dotenv()` is called every time `environment_variables()` is invoked. Move this to a once-per-process initialization (e.g., in `main()` or using `OnceLock`) to avoid redundant `.env` file reads on every docker-compose generation or container operation

```suggestion
        // .env loading should happen once at startup, not here
```

How can I resolve this? If you propose a fix, please make it concise.

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.

2 participants