-
Notifications
You must be signed in to change notification settings - Fork 53
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
Allow Custom party_id for Server derived Client Instance #205
Comments
Hi, First, sorry that I'm responding so long after you posted the feature request! I agree having the party ids consistently be numeric is good for interoperability. In terms of the documentation, as you have probably noticed the Server-side is not nearly as thoroughly documented as the client-side which makes the server op-id issue opaque. Currently I'm working on improving the server-side docs and adding them to the JSDoc site, which will help a little for people in the future who are using the server as a compute party. The original intent of the 's1' party ID is that the server will never have an id that conflicts with an id a client is assigned or requests (as clients will only ever send numeric ids). It also ensures clients are clear that the server is also acting as a compute party, although I'm not sure that's strictly necessary. There are certainly also ways to prevent conflicting IDs between the clients and server that with careful checks and ID assignment. The only other consideration for adding this feature is that there is only one coordination server per computation, so it may not be necessary to have the server compute instance accept arbitrary numeric IDs in the options, it could be a fixed numeric value. 's1' is also used by the server to simplify sending/routing messages to itself, which is the only other place I anticipate having to be careful if the server's ID is going to change, the check there could just be for whatever the server-compute's ID is. |
Hello, I understand that we all are busy. I understand the intent behind 's1' and it makes perfect sense to simplify potential ID conflicts. As far as I could see, searching for it in the repositories code, apart from the initialization and the routing you mentioned, the ID is not used anywhere. However, given that this routing "shortcut" is used in the server portion of the code it is more tricky to have a dynamic id. Are there any plans to support multiple concurrent computations per one server instance? Because I think in that case this shortcut will also lead to problems. I don't know your internal roadmap, or if this multi-computation support is even something you want, but I think it would be a neat feature that goes well with having multiple coordinators in the network as envisioned in issue #9 For now I think I will just change my DB structure as it is the easiest fix. Thank you very much for the explanation and I will eagerly await the server documentation. Is there something I can help with in term of writing the server docs? |
I'm submitting a ...
Do you want to request a feature or report a bug?
Request a feature
Currently when getting a compute (client) instance from the server instance, the id it receives is fixed to 's1'. See compute.js of the server instance for the behavior.
I think that it should be possible to provide the id as part of the options. This would also improve the consistency of the code with the documentation. As of now the documentation states ""party_id": number" for the options object which is actually violated by the default id 's1'.
I think a simple check if the option is provided with a default fallback might be the best suitable solution. I would be willing to try my first pull request ever as it seems to be simple enough. If I missed something and there is a reason for this behavior then I could not see it and would welcome explanation.
I think the change would not only make JIFF more compatible with the documentation but also improve interoperability. For example I write some information, including party ids, into a database and based on the documentation it is written as in integer into the DB. When the server-compute instance provides the id the operation fails. Of course I can easily change it write everything as string but then the behavior kind of contradicts the JIFF documentation when I load the data from the database which would require me to add an additional typecasting layer.
The text was updated successfully, but these errors were encountered: