infinyon / fluvio-client-node

Node.js client for Fluvio
https://infinyon.github.io/fluvio-client-node/
Apache License 2.0
9 stars 10 forks source link

PartitionConsume.fetch should have an optional parameter of Offset #48

Closed simlay closed 3 years ago

simlay commented 3 years ago

Right now fetch requires an Offset instance, this should be an optional parameter which defaults to new Offset({index: 0, from: OffsetFrom.Beginning}) when passed into the rust.

simlay commented 3 years ago

@ajhunyady, @sehz this was talked about a little last week. Making the offset be optional is simple. What the default value should be is unclear - do you want from to be from the beginning or end of the stream?

ajhunyady commented 3 years ago

@simlay it should be from the end to be consistent with the CLI.

simlay commented 3 years ago

Sounds good.

ajhunyady commented 3 years ago

That being said, we should have 2 convenience offsets, one from the beginning and one from the end. https://github.com/infinyon/fluvio-demo-apps-node/blob/master/chat-app/chat-server/src/fluvio-util.ts I found myself using both.

@simlay, ideally, we should provide all 3 convenience functions/constants to be able to remove this file.

sehz commented 3 years ago

This is how it is implemented in previous version: https://github.com/infinyon/fluvio-client-node/commit/cca8e0fd1a3c57e1a2a3f032209b943d1aa88719#diff-25a6634263c1b1f6fc4697a04e2b9904ea4b042a89af59dc93ec1f5d44848a26.

It accepts following format: