Repository navigation
rosmar changes prior to CBG-5041 - #96
Merged
Merged
Conversation
RIT3shSapata
commented
Sep 17, 2026
RIT3shSapata
commented
Contributor
- change rosmar's DCP behvaior to remove checkpoints on one shot DCP feed
- add test coverage for the same
- modify behaviour of other tests
- change rosmar's DCP behvaior to remove checkpoints on one shot DCP feed - add test coverage for the same - modify behaviour of other tests
torcolvin
reviewed
Sep 18, 2026
The checkpoint is shared by the collections in a feed, so StartDCPFeed drops it once they have all drained to the end, rather than each feed doing so in run(). A terminated feed keeps it so the next run can resume. Guards against a nil metadata store. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
errors.AsType needs Go 1.26 or later, so the go directive and the CI toolchain move up together. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
No released golangci-lint targets Go 1.27, so the directive stays at the lowest version errors.AsType compiles on. The terminated feed tests now step one event at a time, so the feed yields and the terminator always closes the queue first. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
torcolvin
reviewed
Sep 24, 2026
torcolvin
left a comment
torcolvin
left a comment
Collaborator
There was a problem hiding this comment.
Most of these comments were generated with claude's code review tool and I tweaked a few of them.
I didn't provide regression tests for each one, but you should verify with a test and then do a fix. The fixes are just suggestions, you can do them in slightly different ways. It is possible not all of them are real bugs that you could hit.
Feeds on different collections can share a checkpoint document. A dump now saves it once, after all of its collections stop. A resumed dump that streamed everything removes only its own entries, and deletes the document when none are left. A dump that did not resume never removes entries. A feed that stops on a corrupt event now counts as cut short. pull() reports a closed queue under the same lock, so a stop after the end-of-feed marker no longer marks a completed feed as terminated. A FeedResume feed with no MetadataStore fails to start, and other feeds with no store skip the checkpoint instead of panicking. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
torcolvin
reviewed
Sep 25, 2026
A feed that names a collection twice now runs one feed on it. Two feeds on the same collection delivered each document twice, and the second feed's done channel replaced the first one's. A feed now checks its Terminator before each event. Before, it stopped only when a separate goroutine closed the queue. If that goroutine ran late, a terminated dump reached its end and counted as completed. The multi-collection test also waits for collection1's backfill to end, because the feed count alone can be 1 before that feed starts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
torcolvin
approved these changes
Sep 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.