Define default URL for chatbot API#3087
Define default URL for chatbot API#3087openshift-merge-bot[bot] merged 1 commit intoopenshift-assisted:masterfrom
Conversation
WalkthroughA new environment variable, Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
openshift-ci
bot
added
approved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/assisted-ui/deploy/start.sh (1)
5-5: Inner single-quotes become part of the valueThe default string is wrapped in single quotes inside the parameter-expansion expression, e.g. the literal value exported will be
'http://localhost:1234'(quotes included).
If the nginx template already adds its own quotes (or no quotes at all) you may end up with double quoting or a syntax error.-export AIUI_CHAT_API_URL="${AIUI_CHAT_API_URL:-'http://localhost:1234'}" +export AIUI_CHAT_API_URL="${AIUI_CHAT_API_URL:-http://localhost:1234}"This matches the typical pattern for default URL values and avoids surprises.
(The same clean-up could be applied toASSISTED_SERVICE_URL, but that predates this PR.)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/assisted-ui/deploy/start.sh(1 hunks)
🔇 Additional comments (1)
apps/assisted-ui/deploy/start.sh (1)
8-8: Confirm all nginx templates that reference the new variable get it substituted
AIUI_CHAT_API_URLis now included for/deploy/nginx.conf, butenvsubstfornginx_ssl.conf(Line 12) does not list it.
If the SSL template ever references$AIUI_CHAT_API_URL, the variable will be left unresolved at runtime.Action items:
- Check
deploy/nginx_ssl.conffor any$AIUI_CHAT_API_URLoccurrences.- If present, extend Line 12 accordingly:
-envsubst '${HTTPS_CERT_FILE} ${HTTPS_KEY_FILE}' < /deploy/nginx_ssl.conf > "${NGINX_CONF_PATH}" +envsubst '${HTTPS_CERT_FILE} ${HTTPS_KEY_FILE} ${AIUI_CHAT_API_URL}' < /deploy/nginx_ssl.conf > "${NGINX_CONF_PATH}"
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/assisted-ui/deploy/start.sh(1 hunks)
⏰ 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). (6)
- GitHub Check: lint
- GitHub Check: tests
- GitHub Check: translation-files
- GitHub Check: unit-tests
- GitHub Check: circular-deps
- GitHub Check: format
🔇 Additional comments (1)
apps/assisted-ui/deploy/start.sh (1)
8-8: Verify that the nginx template actually contains$AIUI_CHAT_API_URL
envsubstnow substitutes the new variable, but if the placeholder is missing in/deploy/nginx.conf, nothing changes and the addition is dead code.
Please double-check the template or add the placeholder where needed.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/assisted-ui/deploy/start.sh(1 hunks)
⏰ 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). (6)
- GitHub Check: lint
- GitHub Check: tests
- GitHub Check: translation-files
- GitHub Check: unit-tests
- GitHub Check: circular-deps
- GitHub Check: format
🔇 Additional comments (2)
apps/assisted-ui/deploy/start.sh (2)
8-8: Good catch adding the new variable toenvsubstlist
AIUI_CHAT_API_URLis now substituted intonginx.conf, keeping the template rendering deterministic.
Once the quoting issue above is fixed, this invocation is correct.
15-15: Verify whetherenv.jstemplate also needs the Chat API URL
env.template.jsis currently substituted with only$AIUI_OCM_TOKEN.
If the frontend code consumesAIUI_CHAT_API_URL, remember to include it here, e.g.:envsubst '$AIUI_OCM_TOKEN $AIUI_CHAT_API_URL' < "$NGINX_APP_ROOT/src/env.template.js" > "$NGINX_APP_ROOT/src/env.js"Double-check the template before merging.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ammont82, rawagner The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-merge-bot
bot
merged commit 78b5487
into
openshift-assisted:master
2 participants
Summary by CodeRabbit