-
Notifications
You must be signed in to change notification settings - Fork 11.4k
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
Analytics Indexer Operational Refactor #21486
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Skipped Deployments
|
pub fn new(watermarks: HashMap<String, CheckpointSequenceNumber>) -> Self { | ||
Self { watermarks } | ||
} |
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.
Note that I moved this code and also made a small change to the constructor in the same PR. Before it took a Vec(K, V) and converted it to a HashMap<K, V> in the constructor. Now it takes the HashMap directly. This let me easily validate task name uniqueness in the sui-analytics-indexer main function.
Generally looks good to me! I think we probably should wait to land till @sadhansood or @phoenix-o have had a chance to also take a look |
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.
The data ingestion part looks good to me! I'm not very familiar with analytics parameters though, but we can start moving the pipelines to the new binary one by one
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.
LGTM! thanks @nickvikeras!
Description
Changes
Why
Next Steps
Open Questions
Test plan
Ran
test.yaml file contents:
Generated a bunch of files like:
The file contents look fine: