Skip to content

Conversation

@jdhoffa
Copy link
Collaborator

@jdhoffa jdhoffa commented Apr 16, 2025

This PR addresses issues with the query service configuration and naming inconsistency.

Changes:

  • Renamed service from query_runner to query_router across the codebase for consistency
  • Added explicit port configuration for the query router service (8002)
  • Fixed environment variable handling in Docker Compose setup
  • Updated service healthcheck configurations to use appropriate hostnames and variables
  • Added QUERY_ROUTER_PORT to .env.example
  • Added proper dependencies between services with healthchecks
  • Added tower dependency to the service's Cargo.toml
  • Updated documentation in README with accurate examples and usage instructions
  • Removed unnecessary environment variable settings from Dockerfiles

Testing:

All services now start correctly using docker compose up with proper dependency resolution and port configurations.

@jdhoffa jdhoffa changed the title fix query runner fix(query_router): rename query_runner to query_router and fix service configuration Apr 16, 2025
@jdhoffa jdhoffa marked this pull request as ready for review April 16, 2025 16:13
@jdhoffa jdhoffa requested a review from Copilot April 16, 2025 16:13
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR renames the query service from query_runner to query_router and updates its service configuration, environment variables, and documentation. Key changes include:

  • Deleting the old query_runner code and introducing new query_router service code.
  • Updating Docker Compose, Cargo.toml, and CI workflow files to reflect the new naming and configuration.
  • Enhancing error handling and healthchecks in the new query_router service.

Reviewed Changes

Copilot reviewed 7 out of 11 changed files in this pull request and generated no comments.

Show a summary per file
File Description
query_runner/src/main.rs Removed the old query_runner service implementation.
query_router/src/main.rs Added new query_router service code with improved endpoints.
query_router/Cargo.toml Updated package name and dependency versions.
docker-compose.yml Adjusted service definitions and healthchecks for query_router.
README.md Updated instructions and examples to reference query_router.
.github/workflows/ci.yml Modified working directories and service names for CI.
.github/workflows/build.yml Updated build context and tags to reference query_router.
Files not reviewed (4)
  • .env.example: Language not supported
  • api/Dockerfile: Language not supported
  • llm_engine/Dockerfile: Language not supported
  • query_router/Dockerfile: Language not supported
Comments suppressed due to low confidence (1)

query_router/src/main.rs:150

  • The error message instructs to set the 'PORT' environment variable, but the service uses 'QUERY_ROUTER_PORT'. Please update the error message to reference 'QUERY_ROUTER_PORT' for consistency.
tracing::error!("Port {} is already in use. Try setting a different port with the PORT environment variable.", port);

@jdhoffa jdhoffa merged commit 0c8c933 into main Apr 16, 2025
6 checks passed
@jdhoffa jdhoffa deleted the fix_query_runner branch April 16, 2025 16:15
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