xline-kv / Xline

A geo-distributed KV store for metadata management
https://xline.cloud
Apache License 2.0
562 stars 71 forks source link

refactor: remove DB generic #806

Closed bsbds closed 1 week ago

bsbds commented 1 month ago

Please briefly answer these questions:

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 86.32479% with 16 lines in your changes missing coverage. Please review.

Project coverage is 75.70%. Comparing base (e35b35a) to head (d836804). Report is 117 commits behind head on master.

Files Patch % Lines
crates/xline/src/server/command.rs 64.70% 3 Missing and 3 partials :warning:
crates/xline/src/server/maintenance.rs 62.50% 1 Missing and 5 partials :warning:
crates/xline/src/server/xline_server.rs 89.65% 0 Missing and 3 partials :warning:
crates/xline/src/storage/db.rs 94.73% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #806 +/- ## ========================================== + Coverage 75.55% 75.70% +0.15% ========================================== Files 180 187 +7 Lines 26938 27822 +884 Branches 26938 27822 +884 ========================================== + Hits 20353 21063 +710 - Misses 5366 5470 +104 - Partials 1219 1289 +70 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

kamyuentse commented 1 month ago

Though xline currently has only one StorageApi impl,the generic is make sense for the community if someone want to use another storage engine.

bsbds commented 1 month ago

Though xline currently has only one StorageApi impl,the generic is make sense for the community if someone want to use another storage engine.

The engine crate already provides a level of abstraction, I think introducing the StorageApi would be redundant.

bsbds commented 3 weeks ago

If we remove the DB generic here, is it necessary to keep StorageApi trait?

removed