Skip to content

Code quality #1931

@iMicknl

Description

@iMicknl

Here’s a focused V2 review with breaking-change/optimization opportunities, based on current code:

  • Avoid 204 JSON parsing errors: __get()/__post() always call response.json(), even after a 204; this can raise. Consider returning None on 204 or letting check_response() return a sentinel and branching before parsing in client.py.

  • Harden error handling: check_response() relies on string matching of error messages. In V2, map errorCode + status to specific exceptions and fall back to a generic error. This reduces brittleness across API variants in client.py.

  • Make payload conversion cheaper/safer: prepare_payload() walks structures twice (camelize + fix abbreviations). For V2, consider moving the “deviceURL” rename into model to_payload() logic or using a single-pass transformer to reduce allocations in serializers.py.

  • Action queue merges mutate inputs: ActionQueue.add() extends Action.commands in-place, which can surprise callers. For V2, copy Action (or its commands) before merging; also replace the O($n^2$) scan with a dict keyed by device_url for faster batching in action_queue.py.

  • Auth strategy imports are heavy: boto3 and warrant_lite import at module load, even if unused. For V2, lazy-import in strategies to reduce startup cost and enable optional extras in strategies.py.

  • Modern async thread offloading: NexityAuthStrategy uses get_event_loop() and run_in_executor. In V2, prefer asyncio.to_thread() or get_running_loop() for Python 3.10+ clarity in strategies.py.

  • Reduce mutation side effects: obfuscate_sensitive_data() mutates the original payload. Consider returning a deep-copied/cleaned structure or adding a copy=True option so callers don’t accidentally mutate cached API payloads in obfuscate.py.

  • Normalize caching behavior: get_setup(), get_devices(), and get_gateways() cache forever unless refresh=True. For V2, add TTL-based caching and an explicit invalidate_cache() to make lifetime predictable in client.py.

  • Use fullmatch + compiled regex: DeviceIdentifier.from_device_url() uses re.search() with a permissive pattern. For V2, precompile with re.compile() and use fullmatch() to reject malformed URLs in models.py.

  • Make State casting safer: EventState applies json.loads() for JSON types and assumes valid JSON strings. Consider catching JSONDecodeError and leaving raw values intact or raising a clearer ValueError in models.py.

  • Use slots for models: Most models are @define(init=False) without slots. Enabling slots=True across models can reduce memory and speed attribute access in V2; it’s a breaking change if users rely on dynamic attributes in models.py.

  • Tighten config typing: create_server_config() uses # type: ignore and accepts Server | str | None. For V2, normalize input earlier (or use a factory) to remove type: ignore and make validation explicit in utils.py.

  • Clarify API return types: get_places() returns a Place, but the docstring says “List the places.” If V2 is a breaking release, either return a list or rename to get_root_place() to remove ambiguity in client.py.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions