sysrepo / sysrepo-python

Python bindings for sysrepo
BSD 3-Clause "New" or "Revised" License
27 stars 21 forks source link

Connection and session access in application callbacks #6

Closed chaichairiko closed 4 years ago

chaichairiko commented 4 years ago

I am using the asyncio functionality of subscriptions (which is a very cool feature). The issue is that I am receiving a timeout for the calls when calling them from the client: <nc:error-message xml:lang="en">Callback event "change" with ID 1 processing timed out.</nc:error-message>

After digging into the implementation and how you used sr_process_events(): https://netopeer.liberouter.org/doc/sysrepo/master/subs.html , I was wondering maybe something was missed. I am not 100% with sysrepo, but IMO the "Done" event is not being handled.

rjarry commented 4 years ago

Hi @chaichairiko

Could you please share the python code of the application that makes the subscription with which you have this problem ?

chaichairiko commented 4 years ago

Hi @rjarry Ive managed to isolate the problem and it is not related to asyncio whatsoever. In my callback I am connecting to sysrepo to get_data (I make other connections throughout the call as well but this initial one is the relevant). While the entire callback (which does a lot of work, including set_items, etc) takes 0.02 seconds. This initial connection take 4.98 seconds. This long delay causes the timeout in the "changes" event.

My question is how to deal with this since it happens all the time. Should I create a connection externally and send it to all the callbacks? Can the code be changed so that the callback receives the session (like the callbacks in the original sysrepo)?

I am still learning how sysrepo works, so I apoligize for the initial issue which is not relevant

rjarry commented 4 years ago

Hi @chaichairiko

You should only create one connection per process. My advice is to create one and store it in a global variable at startup.

After which you can create sessions when needed (in your callbacks or whatever).

chaichairiko commented 4 years ago

@rjarry What about adding the session as the initial parameter in the callback as being done in sysrepo.c? The session is available when subscribing, and can we sent in the callback and used there (it worked in a local branch where I've tried it)

rjarry commented 4 years ago

Unfortunately, this session is an "implicit session" which is created especially for the callbacks and closed immediately when the callback returns.

This makes it impossible to pass the session to async callbacks due to the way they work internally in sysrepo. See https://github.com/sysrepo/sysrepo/issues/1891 for more details. For API consistency, the session argument is not passed to any python callbacks, async nor regular.

The only option is to make the connection accessible to your callbacks. Either via a global variable or via the private_data argument.

chaichairiko commented 4 years ago

Thanks for the detailed response! I will implement a global connection