Skip to content

Make text2sql service endpoint configurable at runtime.#2067

Merged
xiguiw merged 3 commits intoopea-project:mainfrom
ichbinblau:dbqna
Jun 13, 2025
Merged

Make text2sql service endpoint configurable at runtime.#2067
xiguiw merged 3 commits intoopea-project:mainfrom
ichbinblau:dbqna

Conversation

@ichbinblau
Copy link
Copy Markdown

@ichbinblau ichbinblau commented Jun 11, 2025

Description

The PR makes the endpoint to text2sql service configurable as an environment variable APP_TEXT_TO_SQL_URL, which is inspired by ChatQnA's UI implementation. In that case, customers don't need to re-build the docker image.

Issues

1776

Type of change

List the type of change like below. Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds new functionality)
  • Breaking change (fix or feature that would break existing design and interface)
  • Others (enhancement, documentation, validation, etc.)

Dependencies

n/a

Tests

n/a

Copilot AI review requested due to automatic review settings June 11, 2025 09:43
@github-actions
Copy link
Copy Markdown

github-actions bot commented Jun 11, 2025

Dependency Review

✅ No vulnerabilities or license issues found.

Scanned Files

None

Copy link
Copy Markdown
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

Enable runtime configuration of the Text2SQL service endpoint via an APP_TEXT_TO_SQL_URL environment variable so customers can adjust the URL without rebuilding the Docker image.

  • Removed static compile-time binding of the Text2SQL URL in Vite config
  • Added env.sh entrypoint script to replace placeholders in built assets at container startup
  • Updated Docker setup (.env, Dockerfile, docker-compose) to propagate the new APP_TEXT_TO_SQL_URL variable

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
ui/react/vite.config.ts Commented out static define and now passing full process.env
ui/react/env.sh New script loops over APP_ vars and patches built assets
ui/react/.env Changed VITE_TEXT_TO_SQL_URL to placeholder APP_TEXT_TO_SQL_URL
ui/docker/Dockerfile.react Copied env.sh into image and set it as the entrypoint
docker_compose/intel/cpu/xeon/compose.yaml Sets APP_TEXT_TO_SQL_URL using host IP in the service
Comments suppressed due to low confidence (2)

DBQnA/ui/react/vite.config.ts:28

  • Overriding the entire import.meta.env with process.env may embed sensitive environment variables in the client bundle. Prefer explicitly defining only the required VITE_* keys or use a runtime placeholder replacement instead of compile-time define.
      "import.meta.env": process.env,

DBQnA/ui/react/env.sh:5

  • The comment references MY_APP_ but the script greps for APP_. Update the comment to accurately reflect the prefix being used.
for i in $(env | grep APP_) #// Make sure to use the prefix MY_APP_ if you have any other prefix in env.production file variable name replace it with MY_APP_

Signed-off-by: ichbinblau <theresa.shan@intel.com>
Copy link
Copy Markdown
Collaborator

@WenjiaoYue WenjiaoYue left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

ichbinblau added 2 commits June 12, 2025 12:30
Signed-off-by: ichbinblau <theresa.shan@intel.com>
@xiguiw xiguiw merged commit 9923135 into opea-project:main Jun 13, 2025
13 checks passed
alexsin368 pushed a commit to alexsin368/GenAIExamples that referenced this pull request Aug 13, 2025
…#2067)

Signed-off-by: ichbinblau <theresa.shan@intel.com>
Signed-off-by: alexsin368 <alex.sin@intel.com>
cogniware-devops pushed a commit to Cogniware-Inc/GenAIExamples that referenced this pull request Dec 19, 2025
…#2067)

Signed-off-by: ichbinblau <theresa.shan@intel.com>
Signed-off-by: cogniware-devops <ambarish.desai@cogniware.ai>
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.

5 participants