-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
UUIDBuilder -- should it be a singleton as it is in the other language libs? (e.g. up-java
and up-cpp
)
#58
Comments
I'm fine with putting singleton in either |
Bump =^) Hoping to hear feedback from @sophokles73 and @AnotherDaniel in particular |
Sorry for the delay. If I understand correctly, the main point here is that we want to be able to share a single instance of UUIDBuilder within a uEntity for creating UUIDs, right? |
Yup, ideally to prevent the foot-gun of end-users who are developing uEs doing the "wrong" thing by e.g. creating a new
Makes sense. I like the idea of allowing users of Does that make sense? |
A UUID has to be a singleton, otherwise we will have 2 different rand_b generated for the same instance of a running uE (not good at all) |
Agreed for the 99% case. IMHO it'd be nice to have an escape hatch for other cases, e.g. if I wanted to be able to use the (I'm using it to uniquely stamp Rust channel senders so I can then hash them and keep them within a HashMap Actually now that I think about it... that could also be a singleton in that case, since I need only use one.) |
btw, to get actionable:
Agreed on this point -- do you want to take a stab at this or should I? |
Well messages that go in and out of the streamer already have UUIDs in them, there shouldn't be a need to mess around with the IDs. |
Yeah -- not messing with message IDs. Using UUIDBuilder to uniquely stamp the async channels to be able to store them and look them up efficiently. I can walk you through the code sometime. |
just out of curiosity: why is that not good at all? Is there a real problem with a UE producing UUIDs with different random values? They will do that anyway if they are restarted ... |
Be my guest :-) I had already spent some time on this but struggled a lot with the atomic semantics of Rust (or better: my lack of understanding them ;-)). We could always fall back to simply using a Mutex for guarding the counter but I would rather first try to get away with something less strict ... |
We defined in the spec that the random number is used to differentiate an instance of the running uE, if there are 2 random numbers publishing at the same time (same source URI), the system could be confused if we want to correlate messages during a given runtime. |
Hey @sophokles73 -- I took my shot :) Could you take a look at the approach taken in #74? |
Updated UUIDBuilder to be a singleton which exposes only build() to allow end-users a simpler, more consistent API when generating UUIDs for their uE.
@PLeVasseur can this be closed? |
Howdy 👋
@stevenhartley recently brought up on a PR going into
up-client-zenoh-rust
that the other language libs implement their UUID builder / factory as singletons to avoid an end-user accidentally foot-gunning themselves by using multiple UUIDBuilder within one uE.He asked why the same wasn't done in
up-rust
. I had my thoughts as to why:Making it a singleton in Rust forces upon the end user a certain way of interacting with the singleton and there are multiple (reasonable) ways of doing this in Rust. However, maybe it makes sense to give a "simple" path to an end-user not foot-gunning themselves with multiple UUIDBuilder, but allow the possibility for an advanced user to ensure it's a singleton themselves?
A couple of reasonable ways of doing this in Rust:
Looking to hear feedback on this, hopefully from @sophokles73, @AnotherDaniel, and @evshary
The text was updated successfully, but these errors were encountered: