chore: Dev tooling improvements to test ML jobs #1034
Conversation
✅ Deploy Preview for antenna-preview canceled.
|
|
Warning Rate limit exceeded@mihow has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 1 minutes and 55 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis PR expands ignore patterns (.dockerignore, .gitignore), updates VSCode launch profiles, adds a Django management command to process a single image with optional polling, fixes robust POST data handling in a view, and exposes port 5679 in docker-compose for debugging. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as manage.py
participant Cmd as process_single_image.Command
participant DB as Database
participant Queue as Job Queue
participant Poller as Poller
User->>CLI: run process_single_image <image_id> --pipeline <id> [--wait]
CLI->>Cmd: invoke handle()
Cmd->>DB: validate SourceImage, Deployment, Project, Pipeline
DB-->>Cmd: return entities
Cmd->>Queue: submit job (process_single_source_image)
Queue-->>Cmd: return job (pk, task_id, status)
alt --wait provided
Cmd->>Poller: start polling loop (interval ~2s)
loop while not complete
Poller->>Queue: fetch job status/progress
Queue-->>Poller: status/progress
Poller->>User: stream progress & elapsed time
end
Poller->>DB: fetch Detections / Classifications
DB-->>User: final results summary
else no --wait
Cmd->>User: print job created and tip to check status later
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull Request Overview
This PR adds debugging and testing utilities for processing individual images through ML pipelines. It includes Docker configuration for remote debugging, a management command for single-image processing, and a utility function to programmatically submit jobs.
- Added
submit_single_image_jobutility function andprocess_single_imagemanagement command for testing/debugging pipelines on individual images - Configured Docker and VSCode for remote debugging with debugpy on port 5679
- Enhanced
.dockerignorewith comprehensive patterns and fixed request.data handling for non-dict types
Reviewed Changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| ami/jobs/utils.py | New utility function for submitting single-image jobs |
| ami/jobs/management/commands/process_single_image.py | New Django management command for processing individual images with progress monitoring |
| ami/jobs/management/commands/README.md | Documentation for the new management command |
| ami/base/views.py | Fixed potential TypeError when request.data is not a dict |
| docker-compose.yml | Added debugpy port mapping and debugging command example |
| .vscode/launch.json | Added debug configurations for Django and Celery worker attachment |
| .gitignore | Added huggingface_cache directory exclusion |
| .dockerignore | Enhanced with comprehensive patterns and removed duplicate entries |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.dockerignore(1 hunks).gitignore(1 hunks).vscode/launch.json(1 hunks)ami/base/views.py(1 hunks)ami/jobs/management/commands/README.md(1 hunks)ami/jobs/management/commands/process_single_image.py(1 hunks)ami/jobs/utils.py(1 hunks)docker-compose.yml(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
ami/jobs/utils.py (1)
ami/jobs/models.py (3)
MLJob(316-513)logger(989-998)enqueue(801-818)
ami/jobs/management/commands/process_single_image.py (2)
ami/jobs/utils.py (1)
submit_single_image_job(12-78)ami/main/models.py (1)
Classification(2207-2351)
ami/base/views.py (1)
ami/main/api/views.py (1)
get(1626-1682)
🪛 Ruff (0.14.3)
ami/jobs/utils.py
40-40: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
47-47: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
ami/jobs/management/commands/process_single_image.py
44-44: Unused method argument: args
(ARG002)
58-58: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
58-58: Avoid specifying long messages outside the exception class
(TRY003)
65-65: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
65-65: Avoid specifying long messages outside the exception class
(TRY003)
77-77: Do not catch blind exception: Exception
(BLE001)
78-78: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
78-78: Avoid specifying long messages outside the exception class
(TRY003)
78-78: Use explicit conversion flag
Replace with conversion flag
(RUF010)
161-161: Unused noqa directive (non-enabled: E221)
Remove unused noqa directive
(RUF100)
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
ami/jobs/utils.py (1)
40-40: Consider usinglogging.exceptionfor better debugging context.Since exceptions are re-raised,
logging.exceptionwould automatically include the traceback, making debugging easier for this testing/debugging utility.Apply this diff:
except SourceImage.DoesNotExist: - logger.error(f"SourceImage with id {image_id} does not exist") + logger.exception(f"SourceImage with id {image_id} does not exist") raise # Fetch the pipeline and validate it exists @@ except Pipeline.DoesNotExist: - logger.error(f"Pipeline with id {pipeline_id} does not exist") + logger.exception(f"Pipeline with id {pipeline_id} does not exist") raiseAlso applies to: 47-47
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.dockerignore(1 hunks)ami/jobs/management/commands/process_single_image.py(1 hunks)ami/jobs/utils.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .dockerignore
🧰 Additional context used
🧬 Code graph analysis (2)
ami/jobs/utils.py (1)
ami/jobs/models.py (3)
MLJob(316-513)logger(989-998)enqueue(801-818)
ami/jobs/management/commands/process_single_image.py (2)
ami/jobs/utils.py (1)
submit_single_image_job(12-72)ami/main/models.py (1)
Classification(2207-2351)
🪛 Ruff (0.14.3)
ami/jobs/utils.py
40-40: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
47-47: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
ami/jobs/management/commands/process_single_image.py
44-44: Unused method argument: args
(ARG002)
55-57: Avoid specifying long messages outside the exception class
(TRY003)
62-62: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
62-62: Avoid specifying long messages outside the exception class
(TRY003)
69-69: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
69-69: Avoid specifying long messages outside the exception class
(TRY003)
82-82: Do not catch blind exception: Exception
(BLE001)
83-83: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
83-83: Avoid specifying long messages outside the exception class
(TRY003)
83-83: Use explicit conversion flag
Replace with conversion flag
(RUF010)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (2)
ami/jobs/management/commands/process_single_image.py (2)
52-62: Good defensive programming for deployment validation.The explicit check for missing deployment/project (lines 54-57) correctly prevents the
AttributeErrorthat past reviews flagged, and provides a clear, actionable error message.
100-167: Well-implemented wait/poll logic with good UX.The polling loop includes thoughtful details: defensive None checks for the progress chain (lines 112-115), update throttling to avoid spam (line 119), graceful KeyboardInterrupt handling (lines 135-139), and informative result summaries. The implementation provides a smooth user experience for this testing/debugging tool.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
ami/jobs/management/commands/process_single_image.py (1)
127-153: LGTM!The results display provides helpful information for each completion scenario. The late import of
Classificationat line 141 works but could be moved to the top-level imports for consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ami/jobs/management/commands/process_single_image.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
ami/jobs/management/commands/process_single_image.py (2)
ami/ml/orchestration/processing.py (1)
process_single_source_image(11-45)ami/main/models.py (1)
Classification(2207-2351)
🪛 Ruff (0.14.4)
ami/jobs/management/commands/process_single_image.py
32-32: Unused method argument: args
(ARG002)
41-43: Avoid specifying long messages outside the exception class
(TRY003)
48-48: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
48-48: Avoid specifying long messages outside the exception class
(TRY003)
55-55: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
55-55: Avoid specifying long messages outside the exception class
(TRY003)
68-68: Do not catch blind exception: Exception
(BLE001)
69-69: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
69-69: Avoid specifying long messages outside the exception class
(TRY003)
69-69: Use explicit conversion flag
Replace with conversion flag
(RUF010)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (7)
ami/jobs/management/commands/process_single_image.py (7)
1-16: LGTM!The imports and class setup are appropriate for a Django management command.
18-30: LGTM!The command arguments are well-defined with appropriate types and help text.
38-48: LGTM!The image validation properly checks for deployment/project association as addressed in the previous review, and provides clear error messages.
51-55: LGTM!The pipeline validation is straightforward and provides helpful output.
71-78: LGTM!The job creation output provides helpful details for debugging.
80-84: LGTM!Providing a helpful tip for checking job status when not waiting improves the developer experience.
86-125: LGTM!The wait loop implementation is solid:
- Progress checking properly guards against None values (addressing previous review concerns)
- Display updates only when progress/status changes (efficient)
- KeyboardInterrupt is handled gracefully
- Terminal output with
\rprovides good live feedback
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ami/jobs/management/commands/process_single_image.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
ami/jobs/management/commands/process_single_image.py (2)
ami/ml/orchestration/processing.py (1)
process_single_source_image(11-45)ami/main/models.py (1)
Classification(2207-2351)
🪛 Ruff (0.14.4)
ami/jobs/management/commands/process_single_image.py
32-32: Unused method argument: args
(ARG002)
42-44: Avoid specifying long messages outside the exception class
(TRY003)
49-49: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
49-49: Avoid specifying long messages outside the exception class
(TRY003)
56-56: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
56-56: Avoid specifying long messages outside the exception class
(TRY003)
68-68: Do not catch blind exception: Exception
(BLE001)
69-69: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
69-69: Avoid specifying long messages outside the exception class
(TRY003)
69-69: Use explicit conversion flag
Replace with conversion flag
(RUF010)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
ami/jobs/management/commands/process_single_image.py (1)
49-49: Consider explicit exception chaining for clarity.Adding
from errpreserves the original traceback when re-raising, helping debugging. At line 69, you can also usef"{e!s}"instead ofstr(e).Apply this pattern:
except SourceImage.DoesNotExist as err: - raise CommandError(f"SourceImage with id {image_id} does not exist") + raise CommandError(f"SourceImage with id {image_id} does not exist") from errexcept Pipeline.DoesNotExist as err: - raise CommandError(f"Pipeline with id {pipeline_id} does not exist") + raise CommandError(f"Pipeline with id {pipeline_id} does not exist") from errexcept Exception as e: - raise CommandError(f"Failed to submit job: {str(e)}") + raise CommandError(f"Failed to submit job: {e!s}") from eAlso applies to: 56-56, 69-69
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ami/jobs/management/commands/process_single_image.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
ami/jobs/management/commands/process_single_image.py (2)
ami/ml/orchestration/processing.py (1)
process_single_source_image(11-45)ami/main/models.py (1)
Classification(2207-2351)
🪛 Ruff (0.14.4)
ami/jobs/management/commands/process_single_image.py
32-32: Unused method argument: args
(ARG002)
42-44: Avoid specifying long messages outside the exception class
(TRY003)
49-49: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
49-49: Avoid specifying long messages outside the exception class
(TRY003)
56-56: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
56-56: Avoid specifying long messages outside the exception class
(TRY003)
68-68: Do not catch blind exception: Exception
(BLE001)
69-69: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
69-69: Avoid specifying long messages outside the exception class
(TRY003)
69-69: Use explicit conversion flag
Replace with conversion flag
(RUF010)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: lint
- GitHub Check: test
🔇 Additional comments (1)
ami/jobs/management/commands/process_single_image.py (1)
1-153: Well-structured debugging command with past issues resolved.The implementation correctly validates inputs, submits jobs via
process_single_source_image, optionally waits with live progress updates, and displays results. Past critical issues (import path, redundant query, deployment check) have been addressed.
|
TODO @carlosgjs : add a feature flag to re-process all images. Can be either at the project (pydantic) or job level. |
mihow
left a comment
mihow
left a comment
There was a problem hiding this comment.
@carlosgjs thanks for the fixes and debug tools! I pulled out the debugpy parts and merged them in #1048
2 participants
Summary
Management command for testing a single image job + small dev tooling improvements
List of Changes
Test run:
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Chores