Skip to content

Commit dc66c51

Browse files
refactor: Extract duplicate exception handling to helper method in ConfigManager (#66)
The get_instance_name() and get_value() methods had identical exception handling blocks that displayed warnings for various config-related errors. This duplicated ~12 lines of code. Changes: - Add _handle_config_error() helper method to centralize error handling - Update both methods to use a single except clause delegating to helper - Use union type syntax (X | Y) in isinstance checks per UP038 lint rule Co-authored-by: Claude <noreply@anthropic.com>
1 parent b55caaf commit dc66c51

File tree

2 files changed

+57
-17
lines changed

2 files changed

+57
-17
lines changed

progress.md

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -862,3 +862,31 @@ The function calculated monthly cost estimates from hourly prices, but the actua
862862

863863
---
864864

865+
## 2026-01-18: Extract duplicate exception handling in `ConfigManager` to helper method
866+
867+
**File:** `remote/config.py`
868+
869+
**Issue:** The `ConfigManager` class had duplicate exception handling blocks in two methods:
870+
- `get_instance_name()` (lines 255-264)
871+
- `get_value()` (lines 285-290)
872+
873+
Both methods contained identical exception handling code:
874+
```python
875+
except (configparser.Error, OSError, PermissionError) as e:
876+
typer.secho(f"Warning: Could not read config file: {e}", fg=typer.colors.YELLOW)
877+
except (KeyError, TypeError, AttributeError):
878+
typer.secho("Warning: Config file structure is invalid", fg=typer.colors.YELLOW)
879+
except ValueError as e:
880+
typer.secho(f"Warning: Config validation error: {e}", fg=typer.colors.YELLOW)
881+
```
882+
883+
This duplication meant any changes to error handling would need to be made in multiple places.
884+
885+
**Changes:**
886+
- Added new helper method `_handle_config_error(self, error: Exception)` that centralizes the error handling logic
887+
- Updated `get_instance_name()` to catch all config-related exceptions in a single except clause and delegate to the helper
888+
- Updated `get_value()` to use the same pattern
889+
- Reduced code duplication by ~12 lines
890+
891+
---
892+

remote/config.py

Lines changed: 29 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,15 @@ def reload(self) -> None:
241241
self._file_config = None
242242
self._pydantic_config = None
243243

244+
def _handle_config_error(self, error: Exception) -> None:
245+
"""Handle and display config-related errors."""
246+
if isinstance(error, configparser.Error | OSError | PermissionError):
247+
typer.secho(f"Warning: Could not read config file: {error}", fg=typer.colors.YELLOW)
248+
elif isinstance(error, KeyError | TypeError | AttributeError):
249+
typer.secho("Warning: Config file structure is invalid", fg=typer.colors.YELLOW)
250+
elif isinstance(error, ValueError):
251+
typer.secho(f"Warning: Config validation error: {error}", fg=typer.colors.YELLOW)
252+
244253
def get_instance_name(self) -> str | None:
245254
"""Get default instance name from config file or environment variable."""
246255
try:
@@ -252,18 +261,17 @@ def get_instance_name(self) -> str | None:
252261
# Fall back to file config for backwards compatibility
253262
if "DEFAULT" in self.file_config and "instance_name" in self.file_config["DEFAULT"]:
254263
return self.file_config["DEFAULT"]["instance_name"]
255-
except (configparser.Error, OSError, PermissionError) as e:
256-
# Config file might be corrupted or inaccessible
257-
# Log the specific error but don't crash the application
258-
typer.secho(f"Warning: Could not read config file: {e}", fg=typer.colors.YELLOW)
259-
except (KeyError, TypeError, AttributeError):
260-
# Handle malformed config structure
261-
typer.secho("Warning: Config file structure is invalid", fg=typer.colors.YELLOW)
262-
except ValueError as e:
263-
# Handle Pydantic validation errors
264-
typer.secho(f"Warning: Config validation error: {e}", fg=typer.colors.YELLOW)
264+
except (
265+
configparser.Error,
266+
OSError,
267+
PermissionError,
268+
KeyError,
269+
TypeError,
270+
AttributeError,
271+
ValueError,
272+
) as e:
273+
self._handle_config_error(e)
265274

266-
# No configuration found
267275
return None
268276

269277
def set_instance_name(self, instance_name: str, config_path: str | None = None) -> None:
@@ -282,12 +290,16 @@ def get_value(self, key: str) -> str | None:
282290
# Fall back to file config for backwards compatibility
283291
if "DEFAULT" in self.file_config and key in self.file_config["DEFAULT"]:
284292
return self.file_config["DEFAULT"][key]
285-
except (configparser.Error, OSError, PermissionError) as e:
286-
typer.secho(f"Warning: Could not read config file: {e}", fg=typer.colors.YELLOW)
287-
except (KeyError, TypeError, AttributeError):
288-
typer.secho("Warning: Config file structure is invalid", fg=typer.colors.YELLOW)
289-
except ValueError as e:
290-
typer.secho(f"Warning: Config validation error: {e}", fg=typer.colors.YELLOW)
293+
except (
294+
configparser.Error,
295+
OSError,
296+
PermissionError,
297+
KeyError,
298+
TypeError,
299+
AttributeError,
300+
ValueError,
301+
) as e:
302+
self._handle_config_error(e)
291303
return None
292304

293305
def set_value(self, key: str, value: str, config_path: str | None = None) -> None:

0 commit comments

Comments
 (0)