54 adding bulk insert support to telemetrycli - #58
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds bulk CSV insert support to the TelemetryInputCli tool, allowing users to load data into specified tables via a config.yaml schema.
- Introduce
config.yamlandload_table_configs()to map table names to expected CSV columns. - Add
do_bulk_insertCLI command andinsert_data_from_dfhelper to parse and write CSV records. - Update dependencies, README, and
.gitignoreto support YAML configs and CSV handling.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| tools/telemetryInputCli/telemetryInputCli.py | Bulk insert logic, config loader, CSV parsing, and helper methods |
| tools/telemetryInputCli/config.yaml | Table schemas mapping for bulk CSV insert |
| tools/telemetryInputCli/README.MD | Documentation for YAML dependency, .env requirements, and bulk insert config |
| tools/telemetryInputCli/.gitignore | Ignore .csv files |
Comments suppressed due to low confidence (1)
tools/telemetryInputCli/telemetryInputCli.py:122
- [nitpick] Consider adding automated tests for
do_bulk_insertandinsert_data_from_dfto cover key scenarios: valid CSV loads, missing columns, invalid table names, and timestamp conversions.
def do_bulk_insert(self, arg):
| df_config = List[str] | ||
|
|
||
| def load_table_configs() -> Dict[str, df_config]: |
There was a problem hiding this comment.
The df_config alias isn’t necessary; you can use List[str] directly in the function signature. Consider replacing df_config usage with List[str] and removing the alias to simplify the code.
| df_config = List[str] | |
| def load_table_configs() -> Dict[str, df_config]: | |
| def load_table_configs() -> Dict[str, List[str]]: |
| print(f"Invalid unix timestamp: {unix_timestamp}") | ||
| return None |
There was a problem hiding this comment.
This function is annotated to return datetime but returns None on invalid input. Either update the return type to Optional[datetime] or raise an exception to avoid unexpected None values later.
| print(f"Invalid unix timestamp: {unix_timestamp}") | |
| return None | |
| raise ValueError(f"Invalid unix timestamp: {unix_timestamp}. Unix timestamp must be non-negative.") |
| if time is not None: | ||
| data = row.get('temperature') if table == "temp" else row.get('luminosity') | ||
| if pd.notnull(data): | ||
| insertDataIntoWell(well_num, table, data, time, self.payloadTag, self.write_api, self.bucket, self.org) |
There was a problem hiding this comment.
The code passes the table name as the field when calling insertDataIntoWell, but it should use the configured field names (self.wellTempField or self.wellLuminField) so that the InfluxDB field matches the actual data column.
|
|
||
| ### .env file | ||
|
|
||
| The script reuires a file called '.env' with the following format: |
There was a problem hiding this comment.
Fix typo: change reuires to requires.
| The script reuires a file called '.env' with the following format: | |
| The script requires a file called '.env' with the following format: |
|
need more documentation about the csv format (e.g. UNIX timestamp) |
…-bulk-insert-support-to-telemetrycli
|
Sorry about the wait. I've tested it on my end and it works well. The Python script could be cleaned up more but it's just a tool so it's fine for now. I also added some example CSVs for convenience. Thanks Hugo! |
closes #54