-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: update gitignore and remove Jetbrains IDEA files #46
Conversation
Copy pull request changes I proposed to pyunifiprotect Signed-off-by: cyberpower678 <[email protected]>
Signed-off-by: cyberpower678 <[email protected]>
Signed-off-by: cyberpower678 <[email protected]>
Exclude IDE stuff from being committed.
WalkthroughThis update introduces enhancements to the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant UIProtect
participant UnifiProtectSystem
Client->>UIProtect: call download_camera_video(camera_id, filename, ...)
UIProtect->>UnifiProtectSystem: request video data
UnifiProtectSystem-->>UIProtect: return video data
UIProtect-->>Client: return video bytes
Client->>UIProtect: call prepare_camera_video(camera_id, start, end, ...)
UIProtect->>UnifiProtectSystem: request prepared video
UnifiProtectSystem-->>UIProtect: return prepared video data
UIProtect-->>Client: return video info
Client->>UIProtect: call export_camera_video(camera_id, start, end, fps, ...)
UIProtect->>UnifiProtectSystem: request exported video
UnifiProtectSystem-->>UIProtect: return exported video data
UIProtect-->>Client: return video bytes
Client->>UIProtect: call get_camera_video(camera_id, start, end, ...)
alt New Unifi Protect version
UIProtect->>UIProtect: call prepare_camera_video and download_camera_video
else Old Unifi Protect version
UIProtect->>UIProtect: call export_camera_video
end
UIProtect-->>Client: return video data
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
src/uiprotect/api.py (1)
Line range hint
1454-1726
: ### Review of New and Modified Methods inProtectApiClient
download_camera_video
Method (Lines 1454-1516):
- This method handles the downloading of prepared MP4 videos. The implementation supports callbacks and direct file writing which is a good practice for handling large files.
- Coverage Issue: The entire method lacks test coverage. It's crucial to add unit tests to ensure the functionality works as expected and to catch any potential issues early.
- Error Handling: Consider adding more robust error handling around the network requests and file operations to prevent runtime exceptions from unhandled errors.
prepare_camera_video
Method (Lines 1518-1580):
- Prepares a camera video for download. The method includes validation for the camera and channel index which is a good defensive programming practice.
- Coverage Issue: Similar to the
download_camera_video
method, this method also lacks test coverage. Given the complexity and the importance of this method, adding comprehensive tests is essential.- Parameter Validation: The method validates the
channel_index
but does not handle the potentialBadRequest
exception that could be raised from the validation. It's recommended to handle this exception to provide a clearer error message to the caller.
export_camera_video
Method (Lines 1581-1667):
- Exports video directly, similar to the
download_camera_video
method but includes options for time-lapse.- Coverage Issue: This method also lacks test coverage. Given its critical role in video handling, tests are necessary to ensure its reliability and correctness.
- Error Handling: The method could benefit from enhanced error handling, especially around parameter validation and network operations.
get_camera_video
Method (Lines 1669-1726):
- Maintained for backward compatibility. It intelligently decides whether to prepare and download a new video or export it directly based on the Unifi Protect version.
- Deprecated Notice: It's good that a deprecation notice is included. However, it would be beneficial to log a warning when this method is used to alert developers that they are using a deprecated feature.
- Coverage Issue: This method is crucial as it maintains backward compatibility but lacks test coverage. Ensuring this method works correctly is essential to avoid breaking changes for existing implementations.
General Suggestions:
- Test Coverage: All new methods lack sufficient test coverage. It's crucial to add unit tests that cover various scenarios, including edge cases and error conditions.
- Documentation: The documentation is well-written and clear. However, ensuring that it matches the actual implementation and includes all possible exceptions and return types would improve clarity and maintainability.
- Error Handling: Enhancing error handling and adding more specific exceptions could improve the robustness of the API client.
+ # Suggested additions to improve error handling and exception clarity + try: + # existing code + except SpecificException as e: + log.error("An error occurred: %s", e) + raise CustomException("Failed due to ...") from eWould you like assistance in writing the unit tests for these methods? I can help draft some initial test cases or set up a testing framework if needed.
Tools
GitHub Check: codecov/patch
[warning] 1473-1473: src/uiprotect/api.py#L1473
Added line #L1473 was not covered by tests
[warning] 1475-1475: src/uiprotect/api.py#L1475
Added line #L1475 was not covered by tests
[warning] 1485-1485: src/uiprotect/api.py#L1485
Added line #L1485 was not covered by tests
[warning] 1491-1491: src/uiprotect/api.py#L1491
Added line #L1491 was not covered by tests
[warning] 1501-1501: src/uiprotect/api.py#L1501
Added line #L1501 was not covered by tests
[warning] 1503-1503: src/uiprotect/api.py#L1503
Added line #L1503 was not covered by tests
[warning] 1505-1505: src/uiprotect/api.py#L1505
Added line #L1505 was not covered by tests
[warning] 1507-1507: src/uiprotect/api.py#L1507
Added line #L1507 was not covered by tests
[warning] 1509-1509: src/uiprotect/api.py#L1509
Added line #L1509 was not covered by tests
[warning] 1515-1516: src/uiprotect/api.py#L1515-L1516
Added lines #L1515 - L1516 were not covered by tests
[warning] 1546-1547: src/uiprotect/api.py#L1546-L1547
Added lines #L1546 - L1547 were not covered by tests
[warning] 1556-1556: src/uiprotect/api.py#L1556
Added line #L1556 was not covered by tests
[warning] 1561-1562: src/uiprotect/api.py#L1561-L1562
Added lines #L1561 - L1562 were not covered by tests
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- .gitignore (1 hunks)
- src/uiprotect/api.py (6 hunks)
Files skipped from review due to trivial changes (1)
- .gitignore
Additional context used
GitHub Check: codecov/patch
src/uiprotect/api.py
[warning] 1473-1473: src/uiprotect/api.py#L1473
Added line #L1473 was not covered by tests
[warning] 1475-1475: src/uiprotect/api.py#L1475
Added line #L1475 was not covered by tests
[warning] 1485-1485: src/uiprotect/api.py#L1485
Added line #L1485 was not covered by tests
[warning] 1491-1491: src/uiprotect/api.py#L1491
Added line #L1491 was not covered by tests
[warning] 1501-1501: src/uiprotect/api.py#L1501
Added line #L1501 was not covered by tests
[warning] 1503-1503: src/uiprotect/api.py#L1503
Added line #L1503 was not covered by tests
[warning] 1505-1505: src/uiprotect/api.py#L1505
Added line #L1505 was not covered by tests
[warning] 1507-1507: src/uiprotect/api.py#L1507
Added line #L1507 was not covered by tests
[warning] 1509-1509: src/uiprotect/api.py#L1509
Added line #L1509 was not covered by tests
[warning] 1515-1516: src/uiprotect/api.py#L1515-L1516
Added lines #L1515 - L1516 were not covered by tests
[warning] 1546-1547: src/uiprotect/api.py#L1546-L1547
Added lines #L1546 - L1547 were not covered by tests
[warning] 1556-1556: src/uiprotect/api.py#L1556
Added line #L1556 was not covered by tests
[warning] 1561-1562: src/uiprotect/api.py#L1561-L1562
Added lines #L1561 - L1562 were not covered by tests
[warning] 1626-1627: src/uiprotect/api.py#L1626-L1627
Added lines #L1626 - L1627 were not covered by tests
[warning] 1642-1642: src/uiprotect/api.py#L1642
Added line #L1642 was not covered by tests
[warning] 1703-1703: src/uiprotect/api.py#L1703
Added line #L1703 was not covered by tests
[warning] 1705-1705: src/uiprotect/api.py#L1705
Added line #L1705 was not covered by tests
Description of change
This is a simple cleanup pull to prevent accidental commits of IDE workspace files from being made to the repo and removes some Jetbrains IDEA files already on the repo.
Pull-Request Checklist
main
branchFixes #0000
Summary by CodeRabbit
New Features
Enhancements
Deprecated
get_camera_video
method; it now serves as a wrapper for new video preparation and download methods based on the Unifi Protect version.