Accessing SysrepoSession info from callback

[nicolasmorini]

Hi @rjarry!

To be able to implement some niche use cases, I found myself in the need to have access to the SysrepoSession in some of my callbacks (be it a module_change_callback, an oper_data_callback or an rpc_callback). More precisely, I would need access to the username that triggered the callback (and maybe the session ID).

I do not see a clean way to modify sysrepo-python do this without changing the signature of the callbacks, which would essentially make the change not backwards-compatible.

Did you ever have a similar need? If so, do you have any suggestions or hints on how to go about this?

If indeed there is no backwards-compatible way of making it happen in the current version of the project, would you consider this when updating the project to sysrepo v2.x? (assuming that such a change would also be not backwards-compatible).

Best regards,
Nicolas

[rjarry]

Hi,

indeed the session object is not available in python callbacks. I originally made this on purpose to prevent any use after free in async callbacks. Although the same issue could happen in non-async callbacks if the python callback keeps a reference on the session object. See sysrepo/sysrepo#1891 for more details.

If you need more info passed to callbacks without breaking backward compatibility. We could think of an optional argument passed when subscribing which changes the expected callback signatures. I'm not sure what would be the most elegant way of declaring this however. Maybe something like:

def callback(event, request_id, changes, priv, **info):
    print(info['user'])
    print(info['session_id'])
    ....

session.subscribe_module_change("foo", "/foo:config", callback, extra_info=True)

The additional **info keyword arguments would only be passed if extra_info=True is specified when subscribing.

What do you think?

[nicolasmorini]

Hi @rjarry!

Sorry for the late reply, somehow I missed the notification of your reply.

I like your proposal and I think it's as clean as it can be considering the circumstances. Next week I will have some time to look into this topic and prototype a potential pull request. I'll let you know once I have some code available to review/discuss.

Regards,
Nicolas

[nicolasmorini]

Hi @rjarry!

It took me longer than expected, but here is the pull request: #27

Let me know what you think about it and feel free to propose any changes you consider necessary.

Some thoughts:

• I went with your general idea of subscribing with the extra_info=True argument and then passing additional kwargs when calling the callback. At some point I considered re-purposing the private_data existing callback argument but that felt wrong.
• My early comment about preserving backwards compatibility was not a requirement of mine, but just an assumption that breaking backwards compatibility while implementing this feature was not acceptable in general. If you consider that it is worth it, we can consider a cleaner API (at the risk of breaking backwards compatibility).
• I added unit tests to cover all the additions. Plus I already tested it in my own system and it seems to work as expected

Regards,
Nicolas

[rjarry]

Fixed in #27