services/backup/src/Reactors/server/PullBackupReactor.cpp
69–73 ↗	(On Diff #16180)	Is this correct way of performing this conversion? We have a `Vec<unsigned char>` which contains a sequence of bytes and we create a string from it. Are we using `string` as a sequence of bytes or chars here? If we want to use it as a sequence of bytes, maybe it would be less confusing to use e.g. `std::vector<uint8_t>` or `uint8_t[]`. If we want actual chars, then we should convert it somehow because they were probably encoded in UTF8 on Rust's side.

This revision now requires changes to proceed.Sep 5 2022, 7:21 AM

• karol requested review of this revision.Sep 6 2022, 3:39 AM

• karol added inline comments.

services/backup/src/Reactors/server/PullBackupReactor.cpp
69 ↗	(On Diff #16180)	Rust's `Result` is translated into c++ so if it returns `Ok`, c++ gets the value and if it returns `Err`, c++ throws `std::exception`
69–73 ↗	(On Diff #16180)	I remember we've had this discussion before and in general, it's okay to store "any bytes" in `std::string`. We do this in a lot of places in the code so I'm not sure why we wouldn't do it the same way here.
69–73 ↗	(On Diff #16180)	https://stackoverflow.com/questions/4655206/is-it-valid-to-use-stdstring-to-hold-binary-data-to-avoid-manual-dynamic-mem

tomek added 1 blocking reviewer(s): • jon.

tomek added inline comments.

services/backup/src/Reactors/server/PullBackupReactor.cpp
69–73 ↗	(On Diff #16180)	What's the advantage of `string` over `vector<unsigned char>`? For me using `string` hides the intention of storing binary data, so I think it would be better if we don't do that. We can keep it for now, though. Side note is that the logic of transforming `rust::Vec` into `string` is repeated in a couple of places, so it is a good idea to have a function that would do that.

This revision is now accepted and ready to land.Sep 7 2022, 11:18 AM

services/backup/src/Reactors/server/PullBackupReactor.cpp
69–73 ↗	(On Diff #16180)	I don't think this is a really good place to discuss this. You can create a task for this- as I said, this is used all over the place.