Add commands #49
Add commands #49
Conversation
@barkanido Thanks for the heads-up. Instead of matching on This (#49) shouldn't be in that PR's way though, unless I'm missing something. |
|
This one case fails, haven't dug into it. Other than that, I did rebuild and retest using the python client. Please let me know what you think. @clarity0 @evoxmusic |
redisless/src/storage/in_memory.rs
Outdated
| match self.data_mapper.get(key) { | ||
| Some(RedisMeta {data_type: RedisType::String, ..}) => "string".as_bytes(), | ||
| Some(RedisMeta {data_type: RedisType::List, ..}) => "list".as_bytes(), | ||
| Some(RedisMeta {data_type: RedisType::Set, ..}) => "set".as_bytes(), | ||
| Some(RedisMeta {data_type: RedisType::Hash, ..}) => "hash".as_bytes(), | ||
| None => "none".as_bytes(), | ||
| } |
There was a problem hiding this comment.
use let value_type = match ... here and then return value_type.as_bytes()
redisless/src/storage/mod.rs
Outdated
| @@ -18,6 +18,7 @@ pub trait Storage { | |||
| fn read(&mut self, key: &[u8]) -> Option<&[u8]>; | |||
| fn remove(&mut self, key: &[u8]) -> u32; | |||
| fn contains(&mut self, key: &[u8]) -> bool; | |||
| fn value_type(&mut self, key: &[u8]) -> &[u8]; | |||
There was a problem hiding this comment.
I personally prefer the name type_of for this method but that's whatever
There was a problem hiding this comment.
yeah my first thought was type_of(key) that doesn't return the type of the key, but the type of the value inside it is confusing.
|
Can you add tests to |
works on my machine in |
On my machine the returned ttl is very unstable. I ended up changing the delta to 10x. The test passes now, hopefully it doesn't defeat its purpose :D @barkanido |
|
Anything else? @clarity0 |
|
@tbmreza is it all good for you? (I'll review if yes - please use "draft" mode if you are still working on it). Thank you guys |
|
@evoxmusic yes, all good to me. |
|
It looks like tests are failing @tbmreza - can you check? |
|
That's weird. A different test case (server::tests::mget) is flaky on my machine, not the one that's failing in that CI. What do you suggest me to do? I'm thinking about wrapping the Connection's |
Yeah, tests are designed to panic on failure in rust. Why is it flaky though? Maybe it discovered a bug? |
By flaky I mean I simply rerun the test (without any change) and the test passes. |
|
Oops didn't read your comment correctly. I'll try to look why when I can. |
redisless/src/server/tests/mod.rs
Outdated
| let _ = con | ||
| .send_packed_command( | ||
| cmd("INCRBY") | ||
| .arg("0") | ||
| .arg("500") | ||
| .get_packed_command() | ||
| .as_slice(), | ||
| ) | ||
| .unwrap(); |
There was a problem hiding this comment.
Instead of sending a custom command, u can call con.incr(...) since the redis-rs client does implement the INCRBY command. You can do the same for any other command that is implemented directly by redis-rs, DECRBY is also there. 😄
|
So I made a mistake by reusing the same port number for different cases. That sometimes causes starting the server returns Timeout. |
#4 #5 #28