feat: connection manager public #98
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
HatemMn
force-pushed
the
feat/update_for_findex_server
branch
from
e03c97f to
0b4e6b9
Compare
|
Update : exports now look like this : |
* update key length * remove benches on MR * add deny.toml
Fix/review
feat: fix the bugs feat: no more sync futures in memory ADT test:add generic memory ADT tests + delete deprecated vscode config test:add generic memory ADT tests test:add ccr test (fails) test:add ccr test (works) + expose tests to consumers on test cfg fix:fix import of test fix:re-order tests fix:notyes feat: fix the bugs lol fix: fix the code fix: fix clippy ci fix: vscode folder fix: rip first test test: change async tests fix: ci fix: create test submodule + clean tokio dependency fix: review fixes monday2 fix: review fixes monday3 fix: clippy fix: clippy2 fix: clippy3 fix: review fixes fix: last fixes test:change concurrency test fix:newline issues chore:Update src/in_memory_store.rs Co-authored-by: Théophile BRÉZOT <[email protected]> fix:review fixes fix:review fixes2 fix:review fixes3 fix:review fixes4 fix:review fixes5 feat: implement from/to [u8;LEN] for address type, update tests feat:clean deps and adapt word typing to byte sequence test:change concurrent test signatures feat: expore findex core error type feat: add redis store feat: redis store implemented, working tested ok feat: connection manager OK chore: run cargo fmt to pass the lovely CI fix: deny rsa bug fix: cargo deny + lints style: rename redis-store to redis-mem feat: put alltogether memories chore: merge fix : errors + review fixes fix more fixes style: nightly formatting chore: delete and rename various files review unmodify lib.rs rename without _store remove test_utils feature fix: cleanup deny.toml and delete docker-compose.yml (#97) * fix: cleanup deny.toml and delete docker-compose.yml * fix: remove .rustfmt.toml feat: connection manager public feat: change clear db name for less ambigious for server feat: export err type feat: update exports fix: fix some bug fix: fix the memory imports for good fix: an fix chore: reexpose encodings fix: key legth chore: a comment
5 tasks
HatemMn
commented
Dec 27, 2024
src/memory/redis.rs
Outdated
Comment on lines
29
to
35
| #[derive(Clone)] | ||
| pub struct RedisMemory<Address, Word> { | ||
| manager: ConnectionManager, | ||
| script_hash: String, | ||
| a: PhantomData<Address>, | ||
| w: PhantomData<Word>, | ||
| pub manager: ConnectionManager, // is Send + Sync : https://docs.rs/redis/latest/redis/aio/struct.ConnectionManager.html#impl-Send-for-ConnectionManager | ||
| script_hash: String, // trivially Send + Sync | ||
| a: PhantomData<Address>, // is send + Sync because the underlying address we will use in findex is Send + Sync ([u8; ADDRESS_LENGTH]) | ||
| w: PhantomData<Word>, // same as above | ||
| } |
There was a problem hiding this comment.
about this code :
#[derive(Clone)]
pub struct RedisMemory<Address, Word> {
pub manager: ConnectionManager, // is Send + Sync : https://docs.rs/redis/latest/redis/aio/struct.ConnectionManager.html#impl-Send-for-ConnectionManager
script_hash: String, // trivially Send + Sync
a: PhantomData<Address>, // is send + Sync because the underlying address we will use in findex is Send + Sync ([u8; ADDRESS_LENGTH])
w: PhantomData<Word>, // same as above
}
Manuthor
reviewed
Jan 10, 2025
| @@ -1,7 +1,7 @@ | |||
| pub(crate) mod error; | |||
| #[cfg(any(test, feature = "bench", feature = "redis-mem"))] | |||
There was a problem hiding this comment.
Suggested change
| #[cfg(any(test, feature = "bench", feature = "redis-mem"))] |
| @@ -98,6 +100,7 @@ pub(crate) mod tests { | |||
|
|
|||
| /// Adding information from different copies of the same vector should | |||
| /// be visible by all copies. | |||
| #[allow(clippy::redundant_pub_crate)] // false positive. Used in ovec.rs. | |||
There was a problem hiding this comment.
I would rather remove all the #[allow(clippy::redundant_pub_crate)] and remove the - id: clippy-autofix-unreachable-pub line in .pre-commit-config.yaml.
Also please revert all changes where pub(crate) has been changed to pub.
There was a problem hiding this comment.
according to what I saw, seems solved on develop branch
Comment on lines
+24
to
25
| #[cfg(all(not(test), not(feature = "redis-mem")))] | ||
| _ => panic!("no other variant"), |
There was a problem hiding this comment.
Suggested change
| #[cfg(all(not(test), not(feature = "redis-mem")))] | |
| _ => panic!("no other variant"), |
Could this be deleted?
Comment on lines
+88
to
91
| #[allow(clippy::redundant_pub_crate)] // false positive. Used in ovec.rs. | ||
| pub(crate) use vector::*; | ||
|
|
||
| mod vector { |
There was a problem hiding this comment.
Suggested change
| #[allow(clippy::redundant_pub_crate)] // false positive. Used in ovec.rs. | |
| pub(crate) use vector::*; | |
| mod vector { | |
| pub mod vector { |
HatemMn
added a commit
that referenced
this pull request
Jan 15, 2025
Basically the updates previously disccussed here : #98
|
Follow up here : #112 Closing. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Goal of this update is to update according to what might be needed - or not in findex-server.
Related PR : Cosmian/findex-server#20
Not merging this until requirements of findex-server are stable