Fix PR #5 CI failures in GCS temp-file handling and watcher batch-mode tests#7
Fix PR #5 CI failures in GCS temp-file handling and watcher batch-mode tests#7
Conversation
Co-authored-by: yhyatt <10474956+yhyatt@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes CI failures from PR #5 by addressing lint violations and three batch-mode test failures related to context manager usage for image opening. The changes aim to make temporary file handling lint-compliant and update test mocks to match the actual code behavior where batch mode uses Image.open as a context manager.
Changes:
- Updated GCS blob download to use context manager for temp file creation (lint compliance)
- Fixed three batch-mode test mocks to support
Image.openas a context manager - Adjusted import ordering in GCS watcher tests to satisfy ruff
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/dmaf/gcs_watcher.py | Converted temp file creation to use context manager for SIM115 lint compliance |
| tests/test_gcs_watcher.py | Updated temp file handling to use context manager and fixed import ordering |
| tests/test_watcher.py | Fixed three batch-mode test mocks to configure Image.open as a context manager |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| blob.download_to_filename(tmp.name) | ||
|
|
||
| logger.debug(f"Downloaded {gcs_path} -> {tmp.name}") | ||
| return Path(tmp.name) |
There was a problem hiding this comment.
The variable tmp is referenced outside its scope. After the context manager exits (line 100), tmp.name is accessed on line 102, but tmp is only defined within the with block. To fix this, capture tmp.name in a variable before the context exits, similar to the pattern used in tests/test_gcs_watcher.py lines 72-74.
| blob.download_to_filename(tmp.name) | |
| logger.debug(f"Downloaded {gcs_path} -> {tmp.name}") | |
| return Path(tmp.name) | |
| tmp_name = tmp.name | |
| blob.download_to_filename(tmp_name) | |
| logger.debug(f"Downloaded {gcs_path} -> {tmp_name}") | |
| return Path(tmp_name) |
- Dockerfile: install dmaf[gcs] to include google-cloud-storage (required for GCS watch directory support added in PR #7) - Firestore composite indexes created for error_events and borderline_events cleanup queries (eliminates WARNING logs)
PR #5 failed CI due to lint violations and three watcher batch-mode test failures introduced by context-manager-based image opening. This updates the affected code/tests to align with current file-handling patterns and test expectations.
Lint-compliant temp file handling
src/dmaf/gcs_watcher.py.SIM115while keeping behavior unchanged.GCS watcher test cleanup
tests/test_gcs_watcher.pyto use context manager semantics.Batch scan test compatibility with
Image.open(...)context usagetests/test_watcher.pyto mockImage.openas a context manager:test_delete_source_enabled_batch_modetest_delete_source_disabled_batch_modetest_delete_source_upload_failure_batch_modeWarning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
photoslibrary.googleapis.com/usr/bin/python python -m pytest tests/ -m not slow -v --cov=dmaf --cov-report=xml --cov-report=term --cov-fail-under=0(dns block)If you need me to access, download, or install something from one of these locations, you can either:
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.