Skip to content
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

Add ability to get filtered next event from changelog #132

Merged
merged 2 commits into from
Nov 20, 2023

Conversation

janderson-seg
Copy link
Contributor

@janderson-seg janderson-seg commented Nov 17, 2023

Added new method to Iterator, similar to Next but adds the ability to only return events for family/table user cares about.

Also cut out some verbose logging from the changelog.

Testing completed successfully via unit tests

@janderson-seg janderson-seg marked this pull request as ready for review November 20, 2023 18:50
@janderson-seg janderson-seg requested a review from a team as a code owner November 20, 2023 18:50
}

// NextForFamilyTable blocks and returns the next event that matches the specified family and table
func (i *Iterator) NextForFamilyTable(ctx context.Context, family, table string) (event Event, err error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so if int-consumer wanted to use this, it would be to open an iterator for every table it wanted to track?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, do you see any issues with that?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, just wanted to clarify since using Next() and NextForFamilyTable() feel pretty incompatible with how they'd behave. My only suggestion would be to perhaps create a filtered_iterator struct with the same Next() api and have family and table as part of its construction. But I won't block this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, the use of Next and NextForFamilyTable together would not make sense and would not be the intended usage model. Perhaps to make this clearer a separate struct would be better. Will modify and update shortly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks great, thank you for doing that

@janderson-seg janderson-seg merged commit 0bef9c2 into master Nov 20, 2023
6 checks passed
@janderson-seg janderson-seg deleted the Add-ctlstore-changelog-notifier branch November 20, 2023 23:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants