-
-
Notifications
You must be signed in to change notification settings - Fork 19
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
feat:opendata upload #660
feat:opendata upload #660
Conversation
metrics integration for https://github.com/TigreGotico/metrics-server-docker or equivalent
WalkthroughThe changes introduce a new method Changes
Sequence Diagram(s)sequenceDiagram
participant IS as IntentService
participant Req as requests (HTTP)
participant API as User API Server
IS->>IS: _emit_match_message (process intent match)
alt Match Found
IS->>IS: Construct data payload
IS->>Req: _upload_match_data(payload)
Req->>API: POST payload
API-->>Req: Response
Req-->>IS: Return response
else No Match
IS->>IS: Log failure
end
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
Note 🎁 Summarized by CodeRabbit FreeYour organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above. 🪧 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 (
|
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: 2
🧹 Nitpick comments (1)
ovos_core/intent_services/__init__.py (1)
358-381
: Consider privacy and performance optimizations.The current implementation sends all utterances to the metrics server, which raises privacy considerations and could generate significant network traffic.
Consider:
- Adding a sampling mechanism to only upload a percentage of interactions
- Ensuring users are informed about data collection
- Implementing the upload asynchronously to avoid blocking the main thread
# Example of asynchronous implementation using ThreadPoolExecutor from concurrent.futures import ThreadPoolExecutor # Create a thread pool at class level self._metrics_executor = ThreadPoolExecutor(max_workers=2) # In the _upload_match_data method: def _upload_match_data(self, utterance, intent, lang, match_data): # ... existing code ... # Sample data (e.g., only send 10% of interactions) if config.get("sampling_rate", 100) < random.randint(1, 100): return # Submit to thread pool instead of blocking self._metrics_executor.submit(self._do_upload, api_url, data, headers) def _do_upload(self, api_url, data, headers): try: response = requests.post(api_url, data=data, headers=headers, timeout=3) LOG.info(f"Uploaded metrics - Response: {response.status_code}") except Exception as e: LOG.warning(f"Failed to upload metrics: {e}")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ovos_core/intent_services/__init__.py
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: end2end_tests (3.11)
- GitHub Check: unit_tests (3.11)
- GitHub Check: license_tests
🔇 Additional comments (2)
ovos_core/intent_services/__init__.py (2)
19-19
: Added the requests library for HTTP communication.This new dependency is correctly imported to support the metrics upload functionality.
346-356
: Good integration of metrics upload into the intent matching flow.The code properly handles both successful matches and failures, providing valuable data points for metrics collection.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #660 +/- ##
==========================================
- Coverage 75.33% 69.53% -5.81%
==========================================
Files 15 15
Lines 3094 1671 -1423
==========================================
- Hits 2331 1162 -1169
+ Misses 763 509 -254
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
* remove mycroft (#439) * breaking!:remove mycroft module * drop cps * Update translations * Increment Version to 1.0.0a1 * Update Changelog * Update skills-essential.txt (#656) add https://github.com/OpenVoiceOS/ovos-skill-diagnostics to default skills * Increment Version to 1.0.0a2 * Update Changelog * Update skills-media.txt (#658) add youtube music to extras list * Increment Version to 1.0.0a3 * Update Changelog * feat:opendata upload (#660) * feat:opendata upload metrics integration for https://github.com/TigreGotico/metrics-server-docker or equivalent * Update ovos_core/intent_services/__init__.py Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * support multiple urls * fix config --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Increment Version to 1.1.0a1 * Update Changelog --------- Co-authored-by: JarbasAI <[email protected]> Co-authored-by: JarbasAl <[email protected]> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
metrics integration for https://github.com/TigreGotico/metrics-server-docker or equivalent
Summary by CodeRabbit