fix:ocp-pipeline>=1.0.8 #621
Conversation
WalkthroughThe changes involve modifications to the Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #621 +/- ##
==========================================
- Coverage 75.33% 72.64% -2.70%
==========================================
Files 15 15
Lines 3094 1550 -1544
==========================================
- Hits 2331 1126 -1205
+ Misses 763 424 -339
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
test/end2end/session/test_ocp.py (1)
505-508: Consider clarifying the repetition of identical messagesThe addition of four consecutive
"mycroft.audio.service.queue"messages in theexpected_messageslist may benefit from a comment explaining the necessity of multiple identical messages to enhance readability and maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
test/end2end/session/test_ocp.py(3 hunks)
🔇 Additional comments (2)
test/end2end/session/test_ocp.py (2)
129-129: Addition of retry logic enhances test robustness
The inclusion of a second "ovos.common_play.SEI.get" message in the expected_messages list ensures that the test accurately handles scenarios where player information is not received on the first attempt, thereby improving the reliability of the test.
146-146: Simulating player info retry with additional message
By adding another "ovos.common_play.SEI.get" at this point, the test simulates the system's behavior of retrying the request for player information when there is no initial response, which aligns with expected functionality.
debugging what changed with latest package versions
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
test/end2end/session/test_ocp.py (1)
505-508: LGTM! Added expected legacy audio service messages.The addition of multiple queue messages accurately reflects the legacy audio service behavior. Consider adding a comment explaining why multiple queue messages are expected for better maintainability.
'mycroft.audio.service.play', # LEGACY api + # Legacy audio service queues multiple tracks in sequence "mycroft.audio.service.queue", "mycroft.audio.service.queue", "mycroft.audio.service.queue", "mycroft.audio.service.queue",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
requirements/requirements.txt(1 hunks)test/end2end/session/test_ocp.py(3 hunks)
🔇 Additional comments (2)
requirements/requirements.txt (1)
8-8: LGTM! Version range update looks good.
The change from fixed version to a range specification follows semantic versioning best practices, allowing for compatible updates while preventing breaking changes.
Let's verify the compatibility of the latest compatible version:
✅ Verification successful
Let me try a different approach to verify the package version information:
Version range update is appropriate and safe
The change to >=1.0.7, <2.0.0 is well-aligned with the package's version history. The latest version is 1.0.9, which falls within this range, and the package follows semantic versioning with stable 1.x releases. This version specification will allow for compatible updates while preventing breaking changes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check latest compatible version and its release date
curl -s "https://pypi.org/pypi/ovos_ocp_pipeline_plugin/json" | \
jq -r '.releases | keys[] | select(test("^1\\.[0-9]+\\.[0-9]+$"))' | \
sort -V | tail -n1 | \
xargs -I {} jq -r --arg v {} '.releases[$v][0].upload_time' \
"$(curl -s https://pypi.org/pypi/ovos_ocp_pipeline_plugin/json)"
Length of output: 86807
Script:
#!/bin/bash
# Check latest version from PyPI using a simpler curl command
curl -s "https://pypi.org/pypi/ovos-ocp-pipeline-plugin/json" | \
jq -r '.info.version'
Length of output: 89
test/end2end/session/test_ocp.py (1)
129-129: LGTM! Enhanced test robustness with retry mechanism.
The addition of retry attempts for player info makes the test more resilient to timing issues and better reflects real-world scenarios where responses might not be immediate.
Also applies to: 146-146
noticed in end2end tests from OpenVoiceOS/ovos-core#621
* fix:player_proxy noticed in end2end tests from OpenVoiceOS/ovos-core#621 * fix workflows
since OpenVoiceOS/ovos-ocp-pipeline-plugin#34 end2end tests started to fail
Summary by CodeRabbit
Summary by CodeRabbit
Bug Fixes
Tests
Chores
ovos_ocp_pipeline_pluginupdated to>=1.0.10, <2.0.0ovos-workshopupdated to>=3.1.2,<4.0.0ovos-utterance-plugin-cancelupdated to>=0.2.3, <1.0.0