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

RPCCapabilities class doesn't display properly for mergeable cases. (side-effect: the equal action is wrong.) #205

Open
kefeimo opened this issue Aug 26, 2024 · 2 comments
Assignees
Labels

Comments

@kefeimo
Copy link

kefeimo commented Aug 26, 2024

Describe the bug
RPCCapabilities class doesn't display properly for mergeable cases. (side-effect: the equal action is wrong.)

To Reproduce
// Affected commit: V10
i.e., https://github.com/eclipse-volttron/volttron-core/blob/v10/src/volttron/types/auth/authz_types.py

// Steps to reproduce the behavior:
Use the following code snippet to reproduce the error

import volttron.types.auth.authz_types as authz

rpc_cap4 = authz.RPCCapability("id2.rpc2")
rpc_cap4.add_param_restrictions("p2", "v2")
rpc_cap4.add_param_restrictions("id", "id2")

rpc_cap1 = authz.RPCCapability("id.rpc1")
rpc_cap2 = authz.RPCCapability("id2.rpc2", {"id": "id2", "param2": "v2"})

# print(f"{rpc_obj_list_2=}")
print(f"{rpc_cap1=}")
print(f"{rpc_cap2=}")
print(f"{rpc_cap4=}")

rpc_obj_list_2 = authz.RPCCapabilities([rpc_cap1, rpc_cap2, rpc_cap4])
# rpc_obj_list_2 = authz.RPCCapabilities([rpc_cap1, rpc_cap2,])
rpc_obj_list_3 = authz.RPCCapabilities()
rpc_obj_list_3.add_rpc_capability(rpc_cap1)
rpc_obj_list_3.add_rpc_capability(rpc_cap2)
rpc_obj_list_3.add_rpc_capability(rpc_cap4)

print(f"{rpc_obj_list_2=}")
print(f"{rpc_obj_list_3=}")
print(f"{(rpc_obj_list_2==rpc_obj_list_3) =}")

result

rpc_cap1=RPCCapability(resource='id.rpc1', param_restrictions={})
rpc_cap2=RPCCapability(resource='id2.rpc2', param_restrictions={'id': 'id2', 'param2': 'v2'})
rpc_cap4=RPCCapability(resource='id2.rpc2', param_restrictions={'p2': 'v2', 'id': 'id2'})
rpc_obj_list_2=RPCCapabilities(rpc_capabilities=[RPCCapability(resource='id.rpc1', param_restrictions={}), RPCCapability(resource='id2.rpc2', param_restrictions={'id': 'id2', 'param2': 'v2', 'p2': 'v2'}), RPCCapability(resource='id2.rpc2', param_restrictions={'p2': 'v2', 'id': 'id2'})], _rpc_dict={'id.rpc1': {}, 'id2.rpc2': {'p2': 'v2', 'id': 'id2'}})
rpc_obj_list_3=RPCCapabilities(rpc_capabilities=[RPCCapability(resource='id.rpc1', param_restrictions={}), RPCCapability(resource='id2.rpc2', param_restrictions={'id': 'id2', 'param2': 'v2', 'p2': 'v2'})], _rpc_dict={'id.rpc1': {}, 'id2.rpc2': {'id': 'id2', 'param2': 'v2', 'p2': 'v2'}})
(rpc_obj_list_2==rpc_obj_list_3) =False

Expected behavior
rpc_obj_list_2 should display the same as rpc_obj_list_3, i.e.,
RPCCapabilities(rpc_capabilities=[RPCCapability(resource='id.rpc1', param_restrictions={}), RPCCapability(resource='id2.rpc2', param_restrictions={'id': 'id2', 'param2': 'v2', 'p2': 'v2'})], _rpc_dict={'id.rpc1': {}, 'id2.rpc2': {'id': 'id2', 'param2': 'v2', 'p2': 'v2'}}). However, it has repetitive items (i.e., RPCCapability(resource='id2.rpc2', param_restrictions={'id': 'id2', 'param2': 'v2', 'p2': 'v2'}), RPCCapability(resource='id2.rpc2', param_restrictions={'p2': 'v2', 'id': 'id2'}))

In addition, (rpc_obj_list_2==rpc_obj_list_3) should be True.

Also pin @craig8 , @shwethanidd for discussion.

@craig8 craig8 closed this as completed Aug 26, 2024
@craig8 craig8 reopened this Aug 26, 2024
@eclipse-volttron eclipse-volttron deleted a comment Aug 26, 2024
@eclipse-volttron eclipse-volttron deleted a comment Aug 26, 2024
@eclipse-volttron eclipse-volttron deleted a comment Aug 26, 2024
@eclipse-volttron eclipse-volttron deleted a comment Aug 26, 2024
@schandrika
Copy link
Contributor

@kefeimo None of the list type data classes (pubsubcapabilities, rpc capabilities, agents etc.) have equals or hash method overwritten. I simply didn't have a need to compare objects directly using custom_list_type_objec1 == custom_list_type_object2. If you overwrite equals, you have to overwrite hash method too. I have to loop through these objects anyway in order to handle string type capabilities (say "vip1.rpc1") and nested dictionary type capabilities (say {"vip2.rpc2": {"param1":"value1"}}) differently when comparing or merging. So simply did not have a need for these methods. Nor did I implement str or repr. If there is a need, we can certainly add these methods

@schandrika
Copy link
Contributor

Just adding a bit more context- these data class are for VOLTTRON core internal purposes only - ie. only admin role can call the AUTH service rpc method that in turn call the data class methods. That is also one of the reason for adding methods as need - i.e. iteratively - as we add end user user feature/vctl subparsers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants
@craig8 @schandrika @kefeimo and others