Skip to content

feat: parameterize embedding service port and update configuration do…#4

Merged
ArnabChatterjee20k merged 3 commits into
masterfrom
embedding-port
Jun 2, 2026
Merged

feat: parameterize embedding service port and update configuration do…#4
ArnabChatterjee20k merged 3 commits into
masterfrom
embedding-port

Conversation

@ArnabChatterjee20k
Copy link
Copy Markdown
Member

…cumentation

What does this PR do?

(Provide a description of what this PR does.)

Test Plan

(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)

Related PRs and Issues

(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)

Have you read the Contributing Guidelines on issues?

(Write your answer here.)

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Jun 2, 2026

Greptile Summary

This PR parameterizes the embedding service's listen port via a new EMBEDDING_PORT environment variable, replacing the previously undocumented BIND_ADDR variable, and adds a Configuration section to the README.

  • src/main.rs: Reads EMBEDDING_PORT (default 3000), parses it to u16 with a clear startup error on invalid input, and binds to 0.0.0.0:<port>.
  • Dockerfile: Hard-codes EXPOSE 3000 (with a comment explaining it is advisory-only) rather than using a build-arg substitution that would not reflect runtime overrides.
  • docker-compose.yml: Port mapping updated to ${EMBEDDING_PORT:-3000}:${EMBEDDING_PORT:-3000} so the compose file honours the same env var.

Confidence Score: 5/5

Safe to merge; changes are limited to startup configuration and documentation with no impact on the embedding logic.

The port-parsing addition is straightforward and addresses the previously flagged raw-string-to-bind issue. The only gap is that port 0 passes the u16 check and would cause the OS to assign an ephemeral port silently, but this edge case is unlikely in practice and does not affect the service's normal operation.

No files require special attention.

Important Files Changed

Filename Overview
src/main.rs Replaces undocumented BIND_ADDR with EMBEDDING_PORT; adds proper u16 parsing with a clear error message. Port 0 is technically accepted but produces undefined bind behavior.
Dockerfile Hardcodes EXPOSE 3000 with an explanatory comment instead of using a build-arg substitution — correct fix for the build-time vs runtime distinction.
docker-compose.yml Port mapping parameterized to ${EMBEDDING_PORT:-3000}:${EMBEDDING_PORT:-3000}; host and container ports are always kept equal, which is the expected default for single-container deployments.
README.md Adds a Configuration section documenting all four environment variables with their defaults. Content is accurate for EMBEDDING_PORT.

Reviews (2): Last reviewed commit: "linting" | Re-trigger Greptile

Comment thread Dockerfile Outdated
Comment thread src/main.rs Outdated
Comment thread README.md
## Configuration

Configured via environment variables (set them in `.env`):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Possible default mismatch for EMBEDDING_MODELS

The table documents the default as nomic, but the Dockerfile's ARG EMBEDDING_MODELS=nomic,bge-small suggests both models are baked into the image during the warmup step. Could you confirm what the actual runtime default is in the Rust library? If the service loads both models by default, the table entry should be updated to nomic,bge-small to match.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@ArnabChatterjee20k ArnabChatterjee20k merged commit 87d0cf2 into master Jun 2, 2026
3 checks passed
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.

1 participant