-
Notifications
You must be signed in to change notification settings - Fork 55
LCORE-175: Flatten data collection config #281
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
LCORE-175: Flatten data collection config #281
Conversation
WalkthroughThis change consolidates separate feedback and transcript storage configurations into a unified Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Endpoint (feedback/query)
participant UserDataCollection (Config)
participant DataCollectorService
User->>Endpoint (feedback/query): Submit feedback/query
Endpoint (feedback/query)->>UserDataCollection: Get feedback/transcript storage path
UserDataCollection-->>Endpoint (feedback/query): Return path (from user_data_dir)
Endpoint (feedback/query)->>FileSystem: Store feedback/transcript
Note over DataCollectorService: Periodic collection
DataCollectorService->>UserDataCollection: Get storage paths and export config
DataCollectorService->>FileSystem: Collect files
DataCollectorService->>RemoteServer: Send tarball (if export enabled)
DataCollectorService->>FileSystem: Cleanup (if enabled)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (15)
🧰 Additional context used🧠 Learnings (12)📓 Common learningslightspeed-stack.yaml (2)Learnt from: onmete Learnt from: onmete tests/integration/test_configuration.py (2)Learnt from: onmete Learnt from: onmete tests/unit/app/endpoints/test_feedback.py (1)Learnt from: onmete src/app/endpoints/query.py (2)Learnt from: onmete Learnt from: onmete src/app/endpoints/feedback.py (1)Learnt from: onmete src/lightspeed_stack.py (2)Learnt from: onmete Learnt from: onmete README.md (1)Learnt from: onmete tests/configuration/lightspeed-stack.yaml (2)Learnt from: onmete Learnt from: onmete tests/unit/app/endpoints/test_query.py (1)Learnt from: onmete src/models/config.py (2)Learnt from: onmete Learnt from: onmete tests/unit/models/test_config.py (2)Learnt from: onmete Learnt from: onmete 🧬 Code Graph Analysis (10)tests/integration/test_configuration.py (2)
tests/unit/runners/test_data_collector_runner.py (2)
src/app/endpoints/query.py (2)
src/app/endpoints/feedback.py (3)
src/lightspeed_stack.py (2)
tests/unit/app/endpoints/test_query.py (1)
src/runners/data_collector.py (2)
src/models/config.py (1)
tests/unit/models/test_config.py (1)
tests/unit/services/test_data_collector.py (1)
🔇 Additional comments (37)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
| """Unit tests for data collector service.""" | ||
|
|
||
| from pathlib import Path | ||
| from unittest.mock import patch, MagicMock |
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.
I wouldn't worry about these changes.
The current data export is not working because content-type is passed as a header instead of a payload.
Also, it should consume just the provided user data dir from the other PR instead of checking data structure - I want to tackle this too.
Given these upcoming refactoring, I had two choices:
- comment all tests until I update the code and verify it works e2e
- let AI to fix the tests to pass
I decided for the second option.
tisnik
left a comment
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.
Don't we need to be prepared for other ways of sending transcripts? I don't think that sticking to Ingress would be the best solution (from config point of view)
|
closing |
Description
Why
How
Needs rebase after #278
Type of change
Related Tickets & Documents
Checklist before requesting a review
Testing
Summary by CodeRabbit
Documentation
Refactor
Bug Fixes
Tests