sysrepo-python icon indicating copy to clipboard operation
sysrepo-python copied to clipboard

subscription: return parent node to oper_state callback

Open avazquezrd opened this issue 2 years ago • 5 comments

We have a Sysrepo plugin that is subscribed as operational data provider to /ietf-hardware:hardware/component/software leaf, which generates a call for each component when /ietf-hardware:hardware node is requested (as expected). The problem is that with the parameters currently available in OperDataCallbackType (xpath, private_data), it is not possible to distinguish between components. Similar problem with interfaces-state module is described here.

The solution proposed in that issue consists of using the parent node that is returned, since in this case the state must also be built on this parent node. This parent node has the path with the key we need, so we can distinguish between components.

This pull request adds the parent node as parameter exposed in operational state request callback.

avazquezrd avatar Jan 24 '23 13:01 avazquezrd

Codecov Report

Attention: Patch coverage is 16.66667% with 5 lines in your changes are missing coverage. Please review.

Project coverage is 66.54%. Comparing base (956e214) to head (587f88c).

:exclamation: Current head 587f88c differs from pull request most recent head 362f5b2. Consider uploading reports for the commit 362f5b2 to get more accurate results

Files Patch % Lines
sysrepo/subscription.py 0.00% 5 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##           master      #45      +/-   ##
==========================================
- Coverage   66.93%   66.54%   -0.40%     
==========================================
  Files           8        8              
  Lines        1113     1064      -49     
==========================================
- Hits          745      708      -37     
+ Misses        368      356      -12     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Mar 02 '23 18:03 codecov[bot]

Hi sorry for the very long delay. I had a second look and I am still not comfortable with breaking this API.

I had an idea: we could add a parent_xpath key in the extra_info keyword arguments of the operational data callback when extra_info=True is set during subscription. That way, the API is preserved and you get the info you need.

What do you think?

rjarry avatar Mar 26 '24 10:03 rjarry

Hi sorry for the very long delay. I had a second look and I am still not comfortable with breaking this API.

I had an idea: we could add a parent_xpath key in the extra_info keyword arguments of the operational data callback when extra_info=True is set during subscription. That way, the API is preserved and you get the info you need.

What do you think?

Hi! I had already forgotten about this issue 😅. I think that it would be enough to distinguish between different leafs in a list, yes.

Another thing I mentioned in my initial comment is that the parent node is also needed to build the state on it, but I think that the parent node can be re-instantiated from XPath without any problem and we could build the state directly on it, right? In that case, I think that your proposal is fine for the use that this parameter should have!

avazquezrd avatar Mar 26 '24 21:03 avazquezrd

Hi!

I have added requested changes, now the parent_xpath is passed through extra_info.

avazquezrd avatar Apr 10 '24 16:04 avazquezrd

Hi @rjarry, any new comments on this? I'm not sure why the GitHub workflows are failing, but it seems more like a setup thing.

avazquezrd avatar May 14 '24 15:05 avazquezrd