Some small problems discovered

[GopherJ]

Hi, currently I'm still using my fork in case that I need some features specific.

So I played with my fork a little bit and discovered some issues. I fixed it on my fork:

1. max_size = -1 will panic
2. max_size should use bytes for counting
3. pipeline should assure atomic, redis-rs support atomic pipeline
4. realtime publish doesn't work because of .query_async::<_, Vec<String>>(conn) but publish command will not return Vec<String>
5. realtime explanation is incorrect. should be {ns}rt:{qname} instead of {ns}:rt:{qname}

Otherwise it's fine and working well

[GopherJ]

I added also serialize and deserialize support.

[GopherJ]

I'll try to open a PR to fix first these issues listed once available. I list all the problems that I discovered in case you found it useful and you want to avoid these issues.

[DavidBM]

Hi! Thanks you very much for all the work. I will wait to the PR and check it all together :)

If you think that you won't be able to make the PR, let me know and I will go one by one checking all issues.

And again, thanks!

[DavidBM]

So, I'm making some fixes to this.

You can find them in the PR: https://github.com/Couragium/rsmq-async-rs/pull/5

I think I solved the points 1, 2, 3 and 5.

But I'm a bit confused with 4. Why do you need the Vec<String> in the publish command? You should subscribe directly using Redis-rs. Maybe I'm getting it wrong, can you explain the problem further?

Btw, thank you for the report!

[DavidBM]

About the seralize/deserialize support, I see that in your fork you added the json decoder/encoder in the library itself. This library cannot assume an encoding format, that is up to the user. So I won't implement that without a way where the user can choose the format (json, protobuf, etc).

[DavidBM]

So! I went through your commits and saw a bit what you did. I replicated some parts of it. Now it should fit your needs too. I will merge the PR.

[GopherJ]

@DavidBM Hi sorry I planed to do this today but seems you made it first! :)

I had a quick review on the changes. I think 1, 2, 3, 4, 5 should all have been solved. serialize and deserialize support is more for convinience for my project. At the beginning I add generics for send_message and receive_message, but I feels it's a little bit inconvinient for users.

So finnally I added T for Rsmq itself, so that things become simpler. If users have more than one type in queues, they may need to use enum or have multiple Rsmq instance

[DavidBM]

Hi! No problem!

I feel compelled to the option of adding a T to the RsmqConnection trait where that T must have implemented something like "from_bytes" and store everything as bytes.

The main blocker, which I don't know how to solve ergonomically, is that String doesn't seems to support TryFrom Vec, &[u8], etc. So the user would be forced to, even if they want to use String, implement the conversion from u8 for their type, which mixed with the trait rules, will force them to wrap the String with another type.

That all seems very un-ergonomically for me. Do you have any idea of how to make it better?

[GopherJ]

@DavidBM From what I know, https://github.com/servo/bincode support binary serialization/deserialization (if this is what you want). It's the same with serde but maybe more space-friendly/memory-friendly. String implemented also Serialize and DeserializeOwned so it should be fine.

However, I believe bincode doesn't support all kind of types. The last time when I tried it together with sled, some types fail to serialize/deserialize.

For my use case, I have only one type and one queue~ So I just made it simple by adding T: Serialize + DeserializeOwned for Rsmq.

[DavidBM]

I meant for compatibility with other clients. Ideally a rsmq client may send a message encoded in any type. Lets say that the js client sends buffer with binary data that represents an image, taking that as String would fail as it is not a utf8 valid blob.

We cannot assume some encoding in Redis, as other clients may be sending other type of data. For that reason, String is not the best type as many bytes combination are not a valid String. We need to store bytes and take out bytes, and they transform to the user's type (a failable conversion). That from the library point of view.

Now, once that is done, if the user wants to give a type that implements Serialize and Deserialize, that is valis as ling as it implements TryFrom Vec and Into Vec.

[DavidBM]

@GopherJ We just published a new version that should help with our last topic. Can you check if that helps you?

[GopherJ]

@DavidBM Hi, it looks good to me, thanks!