feat(cpp): part of low-level bindings, their tests, and e2e CI config#2852
feat(cpp): part of low-level bindings, their tests, and e2e CI config#2852slbotbm wants to merge 15 commits intoapache:masterfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2852 +/- ##
============================================
- Coverage 70.28% 70.26% -0.03%
Complexity 862 862
============================================
Files 1028 1028
Lines 85280 85280
Branches 62656 62666 +10
============================================
- Hits 59940 59921 -19
- Misses 22828 22838 +10
- Partials 2512 2521 +9
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
de27c1c to
4c6fea6
Compare
|
Hey, I'm looking at it right now! Feel free to @ me on Discord as well. |
6fd067d to
95180c7
Compare
|
This PR is ready for review. |
|
@slbotbm Here's my review: What's good:
Additional questions:
ffi::Identifier {
kind,
length: identifier.length,
value: identifier.value,
}why not: |
hubcio
left a comment
There was a problem hiding this comment.
i also noticed that CompressionAlgorithm, Expiry, MaxTopicSize, PollingStrategy, IdKind aredefined and unit-tested, but the e2e tests pass raw strings directly. any reason for that?
I had intended for those structs to be used in the high-level client. Would it be better to use them for the low-level client also? (but in the future we will be defining the stubs for the high-level client's api in iggy.hpp, so including iggy.hpp in the low level client will lead to confusion.) |
@amlel-el-mahrouss I didn't understand the question. There is no "make_ffi" function defined, as far as I know. |
95180c7 to
ccb893e
Compare
|
@slbotbm Hey, so I was proposing a |
|
@amlel-el-mahrouss I did not create a function like |
|
@slbotbm That's a fair assessment, yes let's not pre-optimize the code before it lands in production, thank you for that comment. |
ccb893e to
9273c28
Compare
Which issue does this PR close?
Add part of low-level bindings for iggy-hpp.
Works towards completion of #2763 .
Rationale
To be merged after #2785
What changed?
Adds low-level bindings + tests + e2e CI config. The following functions have been added for the
Clientopaque type:IggyClientBuilder::new()andIggyClient::from_connection_string(connection_string))tests/client/low_level_e2e.cpp -> e2e tests for client
tests/topic/low_level_e2e.cpp -> e2e tests for topic
tests/stream/low_level_e2e.cpp -> e2e tests for stream
NOTE: The PR is big since Cargo.lock itself is 5k LoC. The actual code itself is about 1k LoC.
Local Execution
AI Usage
If AI tools were used, please answer: