openstack/charms.ceph
Code Issues Proposed changes

Update patch set 7

Browse Source 
Patch Set 7: Code-Review-1

(5 comments)

I like the new algorithm, thanks a lot Alex and Robert! So at this point I only have a few "cosmetic" comments. Note that I haven't reviewed the tests yet, but I'll do it on the next iteration, thanks!

Patch-set: 7
Reviewer: Gerrit User 31289 <31289@4a232e18-c5a9-48ee-94c0-e04e7cca6543>
Label: Code-Review=-1
This commit is contained in:
Gerrit User 31289 2021-06-29 15:35:08 +00:00 committed by Gerrit Code Review
parent 88e35c05a0
commit f01991512b
1 changed files with 89 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

89
f5741d8e4f2dc81fb2cb732faa09774e0d9caaa8 Normal file
View File

@ -0,0 +1,89 @@
{
"comments": [
{
"key": {
"uuid": "bea80de4_8a088fa4",
"filename": "charms_ceph/utils.py",
"patchSetId": 7
},
"lineNbr": 578,
"author": {
"id": 31289
},
"writtenOn": "2021-06-29T15:35:08Z",
"side": 1,
"message": "I think \"flattening a tree\" is a different operation (the other function does a kind a flattening but this one does not IMHO).\n\nHere IMHO we are doing a different thing, two things in fact:\n1. getting all nodes from lookup_type (this is a filtering operation)\n2. setting attributes on these nodes, including all their ancestors\u0027 attributes (inheritance of attributes)\n\n@Alex does such an operation have a name you know of? _filter_nodes_and_populate_attributes? _filter_nodes_and_set_attributes?",
"revId": "f5741d8e4f2dc81fb2cb732faa09774e0d9caaa8",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": true
},
{
"key": {
"uuid": "0f323654_fd3f1dec",
"filename": "charms_ceph/utils.py",
"patchSetId": 7
},
"lineNbr": 579,
"author": {
"id": 31289
},
"writtenOn": "2021-06-29T15:35:08Z",
"side": 1,
"message": "I would change the docstring to \"Get all nodes of the desired type, with all their attributes. These attributes can be direct on inherited from ancestors.\"",
"revId": "f5741d8e4f2dc81fb2cb732faa09774e0d9caaa8",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": true
},
{
"key": {
"uuid": "60336a9b_03219859",
"filename": "charms_ceph/utils.py",
"patchSetId": 7
},
"lineNbr": 598,
"author": {
"id": 31289
},
"writtenOn": "2021-06-29T15:35:08Z",
"side": 1,
"message": "I think it should be \"Get a flattened list of nodes of the desired type\"",
"revId": "f5741d8e4f2dc81fb2cb732faa09774e0d9caaa8",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": true
},
{
"key": {
"uuid": "a1e88fea_d7806c27",
"filename": "charms_ceph/utils.py",
"patchSetId": 7
},
"lineNbr": 600,
"author": {
"id": 31289
},
"writtenOn": "2021-06-29T15:35:08Z",
"side": 1,
"message": "I\u0027d say \"list of nodes defined as a dictionary of attributes and children\"",
"revId": "f5741d8e4f2dc81fb2cb732faa09774e0d9caaa8",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": true
},
{
"key": {
"uuid": "0fadd7d0_d7959321",
"filename": "charms_ceph/utils.py",
"patchSetId": 7
},
"lineNbr": 604,
"author": {
"id": 31289
},
"writtenOn": "2021-06-29T15:35:08Z",
"side": 1,
"message": "Should be \"flattened list of nodes\"",
"revId": "f5741d8e4f2dc81fb2cb732faa09774e0d9caaa8",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": true
}
]
}