Conversation
5c00f7a to
6942be7
Compare
|
Questions:
|
cmwhited
left a comment
There was a problem hiding this comment.
I am going to keep reviewing. But I have concerns about the KafkaConfig
like I commented:
- it is currently duplicated between the kafka client crate and the config crate
- it doesn't expose all available connection params, including things like auth, which is concerning
21c4346 to
0d4e966
Compare
LNSD
left a comment
There was a problem hiding this comment.
Please, check my comments 🙂
LNSD
left a comment
There was a problem hiding this comment.
Please, check my comments 🙂
|
As a piece of advice, this is a considerable PR (many logical changes and many files changed). Although it is ok from the POV of having an overall vision of the feature, I would recommend that you work on small incremental PR instead, as big PRs are prone to reworks due to merge conflicts. Also, I recommend that you run the |
1ebcfb2 to
c0da2c5
Compare
LNSD
left a comment
There was a problem hiding this comment.
Please, check my comments 🙂
LNSD
left a comment
There was a problem hiding this comment.
Please, check my comments 🙂
P.S., you’re doing great with this PR, and I really appreciate all the effort you’re putting into it.
13dd570 to
694f2a3
Compare
9664382 to
20c5f01
Compare
20c5f01 to
2d2a59f
Compare
All feedback addressed in subsequent commits. LNSD approved.
Summary
Adds opt-in Kafka event streaming for worker sync progress, enabling real-time monitoring of dataset sync jobs.
sync.started,sync.progress,sync.completed,sync.failed) per-tableSee app-ampd-worker-events.md for full documentation.
raw_datasetcouples the lifecycle timing of all tables because they share a single block stream.job_idprovides job-level view.ProgressCallbacktrait avoids async overhead;WorkerProgressCallbackbridges to async viatokio::spawn.Test plan