Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9d7593b480
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
pecan/src/services/GrafanaService.ts
Outdated
| const GRAFANA_BRIDGE_URL = | ||
| import.meta.env.VITE_GRAFANA_BRIDGE_URL || | ||
| "http://localhost:3001/api/grafana"; | ||
| const GRAFANA_BRIDGE_URL = "https://grafana-proxy.westernformularacing.workers.dev/grafana-bridge/api/grafana"; |
There was a problem hiding this comment.
Hard-coding the bridge endpoint here removes VITE_GRAFANA_BRIDGE_URL support and the localhost fallback, so local, staging, and self-hosted deployments can no longer point to their own bridge and will always call the production worker URL. This is a functional regression from the previous behavior and will fail anywhere that URL is unreachable or not the intended backend.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR changes how the Grafana bridge/proxy base URL is configured in the frontend by hardcoding it to a specific hosted endpoint.
Changes:
- Replaced the environment-variable-based
GRAFANA_BRIDGE_URLconfiguration with a hardcoded Cloudflare Workers URL. - Commented out the previous
import.meta.env.VITE_GRAFANA_BRIDGE_URLand localhost fallback lines.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
pecan/src/services/GrafanaService.ts
Outdated
| const GRAFANA_BRIDGE_URL = "https://grafana-proxy.westernformularacing.workers.dev/grafana-bridge/api/grafana"; | ||
| // import.meta.env.VITE_GRAFANA_BRIDGE_URL || | ||
| // "http://localhost:3001/api/grafana"; |
There was a problem hiding this comment.
GRAFANA_BRIDGE_URL is now hard-coded to a single production URL, which removes the existing ability to configure the bridge via VITE_GRAFANA_BRIDGE_URL (and breaks local/dev deployments). Suggest restoring the env-based configuration with a sensible default/fallback, and only use the worker URL as the default if that’s intended.
| const GRAFANA_BRIDGE_URL = "https://grafana-proxy.westernformularacing.workers.dev/grafana-bridge/api/grafana"; | |
| // import.meta.env.VITE_GRAFANA_BRIDGE_URL || | |
| // "http://localhost:3001/api/grafana"; | |
| const GRAFANA_BRIDGE_URL = | |
| import.meta.env.VITE_GRAFANA_BRIDGE_URL || | |
| "https://grafana-proxy.westernformularacing.workers.dev/grafana-bridge/api/grafana"; | |
| // For local development you can run a local bridge and set: | |
| // VITE_GRAFANA_BRIDGE_URL=http://localhost:3001/api/grafana |
pecan/src/services/GrafanaService.ts
Outdated
| // In production the bridge runs in daq-server-components alongside Grafana. | ||
| // Set VITE_GRAFANA_BRIDGE_URL to the tunnel/public URL of the bridge service. | ||
| // Example: https://grafana-bridge.westernformularacing.org/api/grafana | ||
| const GRAFANA_BRIDGE_URL = | ||
| import.meta.env.VITE_GRAFANA_BRIDGE_URL || | ||
| "http://localhost:3001/api/grafana"; | ||
| const GRAFANA_BRIDGE_URL = "https://grafana-proxy.westernformularacing.workers.dev/grafana-bridge/api/grafana"; | ||
| // import.meta.env.VITE_GRAFANA_BRIDGE_URL || | ||
| // "http://localhost:3001/api/grafana"; |
There was a problem hiding this comment.
The header comment says to set VITE_GRAFANA_BRIDGE_URL, but the code no longer reads it (it’s commented out). Update the comment to match the actual configuration behavior, or re-enable the env var usage so the documentation stays accurate.
pecan/src/services/GrafanaService.ts
Outdated
| // import.meta.env.VITE_GRAFANA_BRIDGE_URL || | ||
| // "http://localhost:3001/api/grafana"; |
There was a problem hiding this comment.
The commented-out fallback lines are indented inconsistently, which makes it look like they belong to a wrapped expression. If you keep these comments, align them with the surrounding code (or remove them) to avoid confusion.
| // import.meta.env.VITE_GRAFANA_BRIDGE_URL || | |
| // "http://localhost:3001/api/grafana"; | |
| // import.meta.env.VITE_GRAFANA_BRIDGE_URL || | |
| // "http://localhost:3001/api/grafana"; |
haoruizhou
force-pushed
the
dev
branch
3 times, most recently
from
92a7572 to
d6f7665
Compare
Use Grafana proxy URL in telemetry CI Adds setActiveDbcText() so external code (e.g. DbcService) can update the active DBC text without reaching into module internals. Add Grafana proxy worker and DBC auto-load
Deploying daq-radio with
|
| Latest commit: |
ba6872f
|
| Status: | 🚫 Build failed. |
No description provided.