Skip to content
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

Batch JsonRpc ? #2386

Open
Cajuteq opened this issue Jan 24, 2024 · 6 comments
Open

Batch JsonRpc ? #2386

Cajuteq opened this issue Jan 24, 2024 · 6 comments

Comments

@Cajuteq
Copy link

Cajuteq commented Jan 24, 2024

Hi, it appears to me that you currently use an internal implementation of JsonRpc that does not take care of batch requests and I happen to be in need for them for performance issue (The Jeedom is on a small board with a limited 4G connection so limiting network calls helps a lot)

Would it be possible to either develop a support for batch requests or transition towards a lib that does so ?

I did some check and 1ma/JsonRpc seems to be quit documented and usable while also being license compatible (MIT).

If you need a PR for doing so, please let us know.

@zoic21
Copy link
Contributor

zoic21 commented Jan 25, 2024

Hello,
I'am not sure it's good idea. With batch order request will take more time and can failed with timeout.

@Cajuteq
Copy link
Author

Cajuteq commented Jan 26, 2024

Hi, thanks for the response, it is however possible to limit the number of requests allowed in a batch to take care of sure scenario with an optional "batchLimit" parameter.
Also, a timeout fail is always possible due to bad network and should already be taken care of in the client side.

@zoic21
Copy link
Contributor

zoic21 commented Jan 29, 2024

Hello,
Yes we can limite but it's impossible to find this limit, it's depend of what you do. I think for your needed it's better to add this by a plugin. With a plugin you can take all request respond fast if no response needed and do actions.

@Cajuteq
Copy link
Author

Cajuteq commented Jan 29, 2024

[EN] Hi, the limit indeed depends on usage but a sensible default that one can customize as a global option as those present in core/config/default.config.ini sould do the trick. A plugin would not be a good way to go, as the jsonRpc api should implement the jsonRpc specification (linked at the top of the Jeedom jsonRpc documentation) and more specificaly :

6 Batch

To send several Request objects at the same time, the Client MAY send an Array filled with Request objects.

The Server should respond with an Array containing the corresponding Response objects, after all of
the batch Request objects have been processed.


[FR] Bonjour, oui la limite dépend de l'usage, mais un paramètre global avec une valeur par défaut dans core/config/default.config.ini, me semble mieux que l'absence d'implémentation. Utiliser un plugin en remplacement de l'api jsonRpc, n'est pas une solution long terme pour ce cas précis car il s'agit d'une partie de l'implementation du protocole jsonRpc. Plus précisement le 6 chapitre de la spécification jsonRpc librement traduite comme suis :

6 Groupement

Pour envoyer plusieurs Requêtes en même temps, le Client PEUT envoyer une Liste de Requêtes.

Le serveur doit repondre avec une Liste contenant les objets de Réponses après avoir traités toutes les Requêtes.

@zoic21
Copy link
Contributor

zoic21 commented Feb 7, 2024

Bonjour,
Jeedom se base sur le protocole jsonrpc mais ne le suis pas a la lettre, c'est une inspiration d'ou le faite qu'on n'implémente pas le chapitre 6.

Clairement cette modification me fait peur elle va être très lourde et impactante pour un usage vraiment restreint (1er demande en 10ans...). Je vais la remonter a l'équipe Jeedom c'est eux qui ont le dernier mot.

@zoic21
Copy link
Contributor

zoic21 commented Feb 12, 2024

Bonjour,
Après discution avec l'équipe Jeedom nous allons voir pour améliorer l'api en 4.5, non pas avec le batch json rpc mais en rendant l'api accessible depuis mqtt ce qui permettra de faire du batch rpc.

@zoic21 zoic21 added this to the 4.5 milestone Feb 12, 2024
@zoic21 zoic21 removed this from the 4.5 milestone Apr 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants