Skip to content

Conversation

@haoruizhou
Copy link
Collaborator

No description provided.

testing pending
Revert "replacing hard coded with env var"

This reverts commit e51510a.

Reapply "replacing hard coded with env var"

This reverts commit 6185e6b.
@haoruizhou haoruizhou requested a review from Copilot September 12, 2025 05:12
@haoruizhou haoruizhou merged commit bfb0c85 into main Sep 12, 2025
4 checks passed
@haoruizhou haoruizhou deleted the optimizing-file-uploader branch September 12, 2025 05: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 pull request refactors hard-coded configuration values to use environment variables, making the system more configurable and deployment-friendly. Additionally, it includes performance optimizations to reduce memory usage and changes log levels to reduce verbosity.

  • Replaces hard-coded InfluxDB credentials and settings with environment variables
  • Adds comprehensive environment variable configuration to Docker Compose files
  • Implements memory optimization changes in the file uploader with generator patterns and reduced concurrency

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
installer/docker-compose.yml Adds environment variable configuration for car-to-influx service including InfluxDB settings and webhook URL
installer/docker-compose.no-slack.yml Adds similar environment configuration but excludes DBC_FILE setting
installer/car-to-influx/listener.py Replaces hard-coded values with environment variables and adds validation, reduces log verbosity
installer/file-uploader/helper.py Optimizes memory usage with generator patterns and reduces concurrency settings
installer/car-to-influx/WFR25-f772b40.dbc Removes old DBC file completely
installer/car-to-influx/WFR25-6389976.dbc Updates DBC file with new signal definitions and expanded fault/balance messaging

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

INFLUX_TOKEN = os.getenv("INFLUXDB_TOKEN")
INFLUX_ORG = os.getenv("INFLUXDB_ORG", "WFR")
INFLUX_BUCKET = os.getenv("INFLUXDB_BUCKET")
DBC_FILE = os.getenv("DBC_FILE")
Copy link

Copilot AI Sep 12, 2025

Choose a reason for hiding this comment

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

These required environment variables lack default values and could cause the application to fail with unclear errors if not set. Consider adding validation or default values for critical configuration.

Suggested change
DBC_FILE = os.getenv("DBC_FILE")
DBC_FILE = os.getenv("DBC_FILE")
if not DBC_FILE:
raise RuntimeError("Required environment variable DBC_FILE is not set. Please set DBC_FILE to the path of your DBC file.")

Copilot uses AI. Check for mistakes.
INFLUXDB_TOKEN: "${INFLUXDB_TOKEN}"
INFLUXDB_URL: "http://influxdb2:8086"
INFLUXDB_ORG: "WFR"
INFLUXDB_BUCKET: "ourCar"
Copy link

Copilot AI Sep 12, 2025

Choose a reason for hiding this comment

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

The DBC_FILE environment variable is missing in the no-slack configuration but is present in the main docker-compose.yml. This inconsistency could cause the listener service to fail when using the no-slack configuration.

Suggested change
INFLUXDB_BUCKET: "ourCar"
INFLUXDB_BUCKET: "ourCar"
DBC_FILE: "${DBC_FILE:-}"

Copilot uses AI. Check for mistakes.
haoruizhou added a commit that referenced this pull request Nov 13, 2025
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