Uh oh!
There was an error while loading. Please reload this page.
minor migration update - #1708
Conversation
if mode is Prism, it will read all stream jsons once
WalkthroughAdds an Changesis_migration Parameter Rollout
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/lib.rs (1)
35-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid exporting the entire
metadatamodule.
pub mod metadataalso exposes unrelated helpers likeupdate_statsandload_daily_metrics, not just the metadata types. If external callers only needLogStreamMetadata/SchemaVersion, prefer selectivepub usere-exports so those internals stay private.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib.rs` at line 35, The public module export in lib.rs is too broad because pub mod metadata exposes unrelated helpers beyond the intended metadata types. Change the library surface to re-export only the needed symbols from the metadata module, such as LogStreamMetadata and SchemaVersion, and keep helper functions like update_stats and load_daily_metrics private to the module.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/migration/mod.rs`:
- Around line 276-281: The Prism prefetch in fetch_or_create_stream_metadata()
is currently discarded, causing an extra metastore read per stream and
duplicating work during run_migration(). Update the Prism branch that calls
PARSEABLE.metastore.get_all_stream_jsons(...) so the returned stream metadata is
actually reused by the rest of fetch_or_create_stream_metadata() instead of
immediately reading again, and ensure the metadata path honors the prefetched
data across the subsequent flow.
---
Nitpick comments:
In `@src/lib.rs`:
- Line 35: The public module export in lib.rs is too broad because pub mod
metadata exposes unrelated helpers beyond the intended metadata types. Change
the library surface to re-export only the needed symbols from the metadata
module, such as LogStreamMetadata and SchemaVersion, and keep helper functions
like update_stats and load_daily_metrics private to the module.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 563c5597-81fc-42eb-a9da-5fc8b1b4726d
📒 Files selected for processing (11)
src/enterprise/utils.rssrc/handlers/http/cluster/mod.rssrc/handlers/http/modal/query/querier_logstream.rssrc/lib.rssrc/metastore/metastore_traits.rssrc/metastore/metastores/object_store_metastore.rssrc/migration/mod.rssrc/prism/home/mod.rssrc/query/mod.rssrc/query/stream_schema_provider.rssrc/storage/object_storage.rs
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
if mode is Prism, it will read all stream jsons once
Fixes #XXXX.
Description
This PR has:
Summary by CodeRabbit
Bug Fixes
New Features