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

Support consumer offsets for times and list topics #1285

Merged
merged 17 commits into from
Jan 26, 2024

Conversation

cwholmes
Copy link
Contributor

This change adds functionality for the consumer for offsetsForTimes and listTopics. These are part of #94.

@cwholmes
Copy link
Contributor Author

@vlovgr @bplommer @aartigao I'm not sure who the correct people to tag for this would be, but I was wondering if this functionality could be reviewed/added. I have a project that is trying to use fs2-kafka and needs the offsets for times functionality. Please let me know if you are not the correct people.

@chaddaksha
Copy link

@vlovgr @bplommer @aartigao Just bumping this thread to see if we can have this incorporated. Thanks in advance!

@aartigao
Copy link
Contributor

Hey, probably next week I can take a look! (quite busy these days with the newborn and all the celebrations...)

@chaddaksha
Copy link

Thanks! No worries. Happy New Year :)

@aartigao
Copy link
Contributor

aartigao commented Jan 6, 2024

CI is not passing for Scala 2.12.

Please, add a compatibility converter for it in fs2.kafka.internal.converters object 🙏🏽

@cwholmes
Copy link
Contributor Author

cwholmes commented Jan 6, 2024

I replaced the usage with something that works for all scala versions. The ci should hopefully be fixed now.

aartigao
aartigao previously approved these changes Jan 6, 2024
Copy link
Contributor

@aartigao aartigao left a comment

Choose a reason for hiding this comment

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

LGTM!

@cwholmes
Copy link
Contributor Author

cwholmes commented Jan 6, 2024

@aartigao Sorry. Had to fix an imports issue in a test, and updated a case to handle nulls more appropriately. Thanks for reviewing.

@aartigao aartigao force-pushed the support-more-topis-operations branch from c981378 to 2b5efcd Compare January 26, 2024 05:36
@aartigao
Copy link
Contributor

@cwholmes Can you run sbt prePR to fix the headers' year? 🙏🏽

After that, if you don't have more changes, I'd be more than happy to merge it and build a release 🚀

aartigao
aartigao previously approved these changes Jan 26, 2024
@cwholmes
Copy link
Contributor Author

It seems the files I added needed the dates on the headers updated. I have added that and things should be working again.

@aartigao aartigao merged commit 4546d25 into fd4s:series/3.x Jan 26, 2024
8 checks passed
@aartigao
Copy link
Contributor

Thanks for the contribution! ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants