KIP 396 ListOffsets #1029
KIP 396 ListOffsets #1029
Conversation
|
|
There was a problem hiding this comment.
Some initial comments for Round 1.
Mostly addresses the code, some comments on test, and example is not reviewed.
For the public API stuff and how OffsetSpec will be used, we can also discuss internally with everyone to arrive at a consensus on all the bindings at once.
There was a problem hiding this comment.
Example is missing from README.md of the examples/ directory, and also the .gitgnore (see how the other examples are added)
I also checked valgrind for memory leaks in the example. No issues
Mostly, great work, thanks @mahajanadhitya !
mahajanadhitya
force-pushed
the
feature/AdminClient-ListOffsets
branch
from
ef70124 to
edf5c53
Compare
There was a problem hiding this comment.
Approved, don't merge until librdkafka bundle is merged.
Good work @mahajanadhitya !
kafka/adminoptions.go
Outdated
|
|
||
| const ( | ||
| // ReadUncommitted IsolationLevel | ||
| ReadUncommitted = IsolationLevel(C.RD_KAFKA_ISOLATION_LEVEL_READ_UNCOMMITTED) |
There was a problem hiding this comment.
As other enums:
| ReadUncommitted = IsolationLevel(C.RD_KAFKA_ISOLATION_LEVEL_READ_UNCOMMITTED) | |
| IsolationLevelReadUncommitted = IsolationLevel(C.RD_KAFKA_ISOLATION_LEVEL_READ_UNCOMMITTED) |
kafka/adminoptions.go
Outdated
| // ReadUncommitted IsolationLevel | ||
| ReadUncommitted = IsolationLevel(C.RD_KAFKA_ISOLATION_LEVEL_READ_UNCOMMITTED) | ||
| // ReadCommitted IsolationLevel | ||
| ReadCommitted = IsolationLevel(C.RD_KAFKA_ISOLATION_LEVEL_READ_COMMITTED) |
There was a problem hiding this comment.
| ReadCommitted = IsolationLevel(C.RD_KAFKA_ISOLATION_LEVEL_READ_COMMITTED) | |
| IsolationLevelReadCommitted = IsolationLevel(C.RD_KAFKA_ISOLATION_LEVEL_READ_COMMITTED) |
kafka/adminapi.go
Outdated
| cPartition := C.rd_kafka_ListOffsetsResultInfo_topic_partition(cResultInfo) | ||
| Topic := C.GoString(cPartition.topic) | ||
| Partition := TopicPartition{Topic: &Topic, Partition: int32(cPartition.partition)} | ||
| Value.Offset = int64(cPartition.offset) |
There was a problem hiding this comment.
| Value.Offset = int64(cPartition.offset) | |
| Value.Offset = kafka.Offset(cPartition.offset) |
There was a problem hiding this comment.
Just using Offset() here since the package is already kafka
|
Addressed comments and merged master. |
No description provided.