)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a2700a057aa2f3845e5739e4fc7226e0feb005bf","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"26680c80_0222b9be","updated":"2023-02-01 18:56:18.000000000","message":"I think my opinion wont be popular but I think half of this commit is re-implementing things that exists out there as a library.","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"086921af1b2b8caf65d1c64860283b38e1c33702","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"e3c6ca17_e294cd3f","updated":"2023-01-24 18:11:30.000000000","message":"Thank you Sylvain for this work. Just some comments.","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"2372bec4eb3ce73817ba6221c1325fed02b20648","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"9541fa3d_13c7a31f","updated":"2023-01-23 15:22:51.000000000","message":"recheck grenade multinode problem","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"4d362177b7e1c11f6469312e0ef1b81d6a6ce0be","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"54393fb0_3e4b2a50","in_reply_to":"26680c80_0222b9be","updated":"2023-02-02 18:03:08.000000000","message":"Well, if python was providing by a stdlib, surely we could use it  but I don\u0027t really want to add a dependency on some Github project while it\u0027s simple to direcly call sysfs.","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"78cd995a0d82d14b832407978e342343ecdfd04e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"669d0f23_2e8f5c6b","updated":"2023-02-01 19:01:19.000000000","message":"the path thing is solved in PS3 the rest of my comments still seems to be valid.","commit_id":"021cc6408c611cce5422398f3c16c070aa78a334"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"61af101843e1ab20d0440fea4195fa27d95e8b87","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"c7609739_b500fc04","updated":"2023-02-03 11:38:05.000000000","message":"recheck bug 2004641","commit_id":"2d2dedb5a1163e56fa1bf5075f6fd8d85ac77168"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d387faf721487153b0d07021c94d53aca64f1fa5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"546b056d_d05e26cb","updated":"2023-02-09 17:31:55.000000000","message":"looks good","commit_id":"ddf96bcd31bad89ffa391251179ba13bb789991d"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"f3bdb9e1c9be366c6056df4755a1d175c7daca81","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"7028828f_9976ba0c","updated":"2023-02-15 08:35:03.000000000","message":"recheck bfv instance creation got timed out in nova-next.","commit_id":"ddf96bcd31bad89ffa391251179ba13bb789991d"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"51fc2877da83acf91c37f87b354503ed4d6e7da8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"6445f303_17946dc6","updated":"2023-02-14 08:57:43.000000000","message":"recheck ceph job has been set non-voting","commit_id":"ddf96bcd31bad89ffa391251179ba13bb789991d"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"5b15489c7ccf5fdfc6fd1011ffe6fbba11140c59","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"8323eb36_ce6777da","updated":"2023-02-14 12:31:01.000000000","message":"recheck seems like a new unit test instability\n\n\nTraceback (most recent call last):\n  File \"/home/zuul/src/opendev.org/openstack/nova/nova/tests/unit/test_wsgi.py\", line 162, in test_server_pool_waitall\n    mock_waitall.assert_called_once_with()\n  File \"/usr/lib/python3.8/unittest/mock.py\", line 924, in assert_called_once_with\n    raise AssertionError(msg)\nAssertionError: Expected \u0027waitall\u0027 to be called once. Called 2 times.\nCalls: [call(), call()].\n\n","commit_id":"ddf96bcd31bad89ffa391251179ba13bb789991d"},{"author":{"_account_id":7634,"name":"Takashi Natsume","email":"takanattie@gmail.com","username":"natsumet"},"change_message_id":"ec903e9e0be3d9969563975c9cd77671e259313d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"ba864406_8d1cbd63","updated":"2023-02-11 02:11:05.000000000","message":"recheck timeout","commit_id":"ddf96bcd31bad89ffa391251179ba13bb789991d"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"467cf0586b217ec18cda0dbbaecf6aa6496c7c6e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"68a5b31c_b20a57c4","updated":"2023-02-10 08:58:56.000000000","message":"recheck unrelated glance tempest test failure.(this is a bug in tempest)","commit_id":"ddf96bcd31bad89ffa391251179ba13bb789991d"}],"nova/conf/libvirt.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"57031804d97230c6df4554f76d0781091a926e33","unresolved":true,"context_lines":[{"line_number":1478,"context_line":"\"\"\"),"},{"line_number":1479,"context_line":"]"},{"line_number":1480,"context_line":""},{"line_number":1481,"context_line":"libvirt_cpu_mgmt_opts \u003d ["},{"line_number":1482,"context_line":"    cfg.StrOpt(\u0027cpu_power_governor_low\u0027,"},{"line_number":1483,"context_line":"               default\u003d\u0027powersave\u0027,"},{"line_number":1484,"context_line":"               help\u003d\u0027Governor to use in order \u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"02d89ce8_b8eae202","line":1481,"updated":"2023-01-24 00:19:47.000000000","message":"based on the sepc dont we also need a config option to opt into this and also to enable the feature\n\nso opt into govenor vs online state and turn this feature on and off\n\n\n——-\nlater \n————-\nok this module is not actullly callled by the driver in this patch so this is actully used by nova yet so its fine to add the other config options later","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"385c0638dfd1fd14465a0e707a1154e42e9d4282","unresolved":false,"context_lines":[{"line_number":1478,"context_line":"\"\"\"),"},{"line_number":1479,"context_line":"]"},{"line_number":1480,"context_line":""},{"line_number":1481,"context_line":"libvirt_cpu_mgmt_opts \u003d ["},{"line_number":1482,"context_line":"    cfg.StrOpt(\u0027cpu_power_governor_low\u0027,"},{"line_number":1483,"context_line":"               default\u003d\u0027powersave\u0027,"},{"line_number":1484,"context_line":"               help\u003d\u0027Governor to use in order \u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"3d2b5044_4037b92b","line":1481,"in_reply_to":"02d89ce8_b8eae202","updated":"2023-01-30 16:09:04.000000000","message":"indeed, I added the needed opt in the next patch as we don\u0027t need it here.","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"086921af1b2b8caf65d1c64860283b38e1c33702","unresolved":true,"context_lines":[{"line_number":1480,"context_line":""},{"line_number":1481,"context_line":"libvirt_cpu_mgmt_opts \u003d ["},{"line_number":1482,"context_line":"    cfg.StrOpt(\u0027cpu_power_governor_low\u0027,"},{"line_number":1483,"context_line":"               default\u003d\u0027powersave\u0027,"},{"line_number":1484,"context_line":"               help\u003d\u0027Governor to use in order \u0027"},{"line_number":1485,"context_line":"                    \u0027to reduce CPU power consumption\u0027),"},{"line_number":1486,"context_line":"    cfg.StrOpt(\u0027cpu_power_governor_high\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"910da1a9_983c1b39","line":1483,"updated":"2023-01-24 18:11:30.000000000","message":"As this option is string you want to indicate and explain what are the possible values, right?\n\nprobably using `choices`?","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"385c0638dfd1fd14465a0e707a1154e42e9d4282","unresolved":false,"context_lines":[{"line_number":1480,"context_line":""},{"line_number":1481,"context_line":"libvirt_cpu_mgmt_opts \u003d ["},{"line_number":1482,"context_line":"    cfg.StrOpt(\u0027cpu_power_governor_low\u0027,"},{"line_number":1483,"context_line":"               default\u003d\u0027powersave\u0027,"},{"line_number":1484,"context_line":"               help\u003d\u0027Governor to use in order \u0027"},{"line_number":1485,"context_line":"                    \u0027to reduce CPU power consumption\u0027),"},{"line_number":1486,"context_line":"    cfg.StrOpt(\u0027cpu_power_governor_high\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"317884ad_d69dd989","line":1483,"in_reply_to":"05339313_0d46299b","updated":"2023-01-30 16:09:04.000000000","message":"indeed, we discussed this is in the spec review and we agreed on not having choices.\n\nResolving this comment as so.","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"077bc43149f134e9b7f7016d44667b8cf0b7c727","unresolved":true,"context_lines":[{"line_number":1480,"context_line":""},{"line_number":1481,"context_line":"libvirt_cpu_mgmt_opts \u003d ["},{"line_number":1482,"context_line":"    cfg.StrOpt(\u0027cpu_power_governor_low\u0027,"},{"line_number":1483,"context_line":"               default\u003d\u0027powersave\u0027,"},{"line_number":1484,"context_line":"               help\u003d\u0027Governor to use in order \u0027"},{"line_number":1485,"context_line":"                    \u0027to reduce CPU power consumption\u0027),"},{"line_number":1486,"context_line":"    cfg.StrOpt(\u0027cpu_power_governor_high\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"05339313_0d46299b","line":1483,"in_reply_to":"86d7ba67_bd55cc46","updated":"2023-01-25 08:24:26.000000000","message":"I probably missed something in the spec.","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"e78d3d778c81262dd60cb74ff9aff8704205b93d","unresolved":true,"context_lines":[{"line_number":1480,"context_line":""},{"line_number":1481,"context_line":"libvirt_cpu_mgmt_opts \u003d ["},{"line_number":1482,"context_line":"    cfg.StrOpt(\u0027cpu_power_governor_low\u0027,"},{"line_number":1483,"context_line":"               default\u003d\u0027powersave\u0027,"},{"line_number":1484,"context_line":"               help\u003d\u0027Governor to use in order \u0027"},{"line_number":1485,"context_line":"                    \u0027to reduce CPU power consumption\u0027),"},{"line_number":1486,"context_line":"    cfg.StrOpt(\u0027cpu_power_governor_high\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"86d7ba67_bd55cc46","line":1483,"in_reply_to":"910da1a9_983c1b39","updated":"2023-01-24 19:08:42.000000000","message":"no we explcitly dont\nwe dont wnat to make this kernel/disto specific\nthat is why we are not using choices here.","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"086921af1b2b8caf65d1c64860283b38e1c33702","unresolved":true,"context_lines":[{"line_number":1483,"context_line":"               default\u003d\u0027powersave\u0027,"},{"line_number":1484,"context_line":"               help\u003d\u0027Governor to use in order \u0027"},{"line_number":1485,"context_line":"                    \u0027to reduce CPU power consumption\u0027),"},{"line_number":1486,"context_line":"    cfg.StrOpt(\u0027cpu_power_governor_high\u0027,"},{"line_number":1487,"context_line":"             default\u003d\u0027performance\u0027,"},{"line_number":1488,"context_line":"             help\u003d\u0027Governor to use in order to have best CPU performance\u0027),"},{"line_number":1489,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":2,"id":"39f27f6e_73e7e8e3","line":1486,"updated":"2023-01-24 18:11:30.000000000","message":"same.","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"385c0638dfd1fd14465a0e707a1154e42e9d4282","unresolved":false,"context_lines":[{"line_number":1483,"context_line":"               default\u003d\u0027powersave\u0027,"},{"line_number":1484,"context_line":"               help\u003d\u0027Governor to use in order \u0027"},{"line_number":1485,"context_line":"                    \u0027to reduce CPU power consumption\u0027),"},{"line_number":1486,"context_line":"    cfg.StrOpt(\u0027cpu_power_governor_high\u0027,"},{"line_number":1487,"context_line":"             default\u003d\u0027performance\u0027,"},{"line_number":1488,"context_line":"             help\u003d\u0027Governor to use in order to have best CPU performance\u0027),"},{"line_number":1489,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":2,"id":"d779d28d_496464e8","line":1486,"in_reply_to":"39f27f6e_73e7e8e3","updated":"2023-01-30 16:09:04.000000000","message":"As well, was discussing on the spec, so resolving this comment.","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"}],"nova/filesystem.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a2700a057aa2f3845e5739e4fc7226e0feb005bf","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"00b273ea_f84fed6b","updated":"2023-02-01 18:56:18.000000000","message":"You have to list the new files in mypy-files.txt to get them checked, then you need to fix the findings :)\n```   \ndiff --git a/mypy-files.txt b/mypy-files.txt\nindex 5a3b9ab339..aab1645dc0 100644\n--- a/mypy-files.txt\n+++ b/mypy-files.txt\n@@ -1,6 +1,7 @@\n nova/compute/manager.py\n nova/compute/pci_placement_translator.py\n nova/crypto.py\n+nova/filesystem.py\n nova/limit/local.py\n nova/limit/placement.py\n nova/network/neutron.py\n@@ -11,6 +12,8 @@ nova/scheduler/request_filter.py\n nova/scheduler/utils.py\n nova/virt/driver.py\n nova/virt/hardware.py\n+nova/virt/libvirt/cpu/__init__.py\n+nova/virt/libvirt/cpu/core.py\n nova/virt/libvirt/machine_type_utils.py\n nova/virt/libvirt/__init__.py\n nova/virt/libvirt/driver.py\n```","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"4d362177b7e1c11f6469312e0ef1b81d6a6ce0be","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"954d5ff8_ba4ff3db","in_reply_to":"00b273ea_f84fed6b","updated":"2023-02-02 18:03:08.000000000","message":"Shit, you\u0027re right.","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"57031804d97230c6df4554f76d0781091a926e33","unresolved":true,"context_lines":[{"line_number":35,"context_line":"    return default"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"},{"line_number":39,"context_line":"def write_sys(path: str, data: str \u003d None) -\u003e ty.Optional[str]:"},{"line_number":40,"context_line":"    try:"},{"line_number":41,"context_line":"        with open(os.path.join(SYS, path), mode\u003d\u0027w\u0027) as fd:"}],"source_content_type":"text/x-python","patch_set":2,"id":"deaec9b4_ba2b3989","line":38,"updated":"2023-01-24 00:19:47.000000000","message":"the contract of this function (any file and any data)\n\nis two broad to have a privsep decorator\n\nyou should move this decorator to the functino that calls this and take the cpu core id as a parmter not the path to that specialied function.\n\nmore on this later","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"19af0bd40142aa37e7fc27441f629c7739aa6213","unresolved":false,"context_lines":[{"line_number":35,"context_line":"    return default"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"},{"line_number":39,"context_line":"def write_sys(path: str, data: str \u003d None) -\u003e ty.Optional[str]:"},{"line_number":40,"context_line":"    try:"},{"line_number":41,"context_line":"        with open(os.path.join(SYS, path), mode\u003d\u0027w\u0027) as fd:"}],"source_content_type":"text/x-python","patch_set":2,"id":"b01ed728_c4f691a4","line":38,"in_reply_to":"deaec9b4_ba2b3989","updated":"2023-01-31 19:11:03.000000000","message":"Ack","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a2700a057aa2f3845e5739e4fc7226e0feb005bf","unresolved":true,"context_lines":[{"line_number":36,"context_line":""},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"},{"line_number":39,"context_line":"def write_sys(path: str, data: str \u003d None) -\u003e ty.Optional[str]:"},{"line_number":40,"context_line":"    try:"},{"line_number":41,"context_line":"        with open(os.path.join(SYS, path), mode\u003d\u0027w\u0027) as fd:"},{"line_number":42,"context_line":"            fd.write(data)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9902ecaf_9cbc396c","line":39,"range":{"start_line":39,"start_character":46,"end_line":39,"end_character":63},"updated":"2023-02-01 18:56:18.000000000","message":"as far as I see this is not returning an optional str. It always returns None","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"4d362177b7e1c11f6469312e0ef1b81d6a6ce0be","unresolved":false,"context_lines":[{"line_number":36,"context_line":""},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"},{"line_number":39,"context_line":"def write_sys(path: str, data: str \u003d None) -\u003e ty.Optional[str]:"},{"line_number":40,"context_line":"    try:"},{"line_number":41,"context_line":"        with open(os.path.join(SYS, path), mode\u003d\u0027w\u0027) as fd:"},{"line_number":42,"context_line":"            fd.write(data)"}],"source_content_type":"text/x-python","patch_set":2,"id":"37bac705_9377d9cc","line":39,"range":{"start_line":39,"start_character":46,"end_line":39,"end_character":63},"in_reply_to":"9902ecaf_9cbc396c","updated":"2023-02-02 18:03:08.000000000","message":"Ack","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a2700a057aa2f3845e5739e4fc7226e0feb005bf","unresolved":true,"context_lines":[{"line_number":41,"context_line":"        with open(os.path.join(SYS, path), mode\u003d\u0027w\u0027) as fd:"},{"line_number":42,"context_line":"            fd.write(data)"},{"line_number":43,"context_line":"    except (OSError, ValueError) as e:"},{"line_number":44,"context_line":"        LOG.error(e)"}],"source_content_type":"text/x-python","patch_set":2,"id":"01380a8f_d1df00ba","line":44,"updated":"2023-02-01 18:56:18.000000000","message":"I haven\u0027t checked the usage of this yet, but by not signalling the error to the caller it might think that the write succeeded even though it is failed. This can be error prone.","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"4d362177b7e1c11f6469312e0ef1b81d6a6ce0be","unresolved":false,"context_lines":[{"line_number":41,"context_line":"        with open(os.path.join(SYS, path), mode\u003d\u0027w\u0027) as fd:"},{"line_number":42,"context_line":"            fd.write(data)"},{"line_number":43,"context_line":"    except (OSError, ValueError) as e:"},{"line_number":44,"context_line":"        LOG.error(e)"}],"source_content_type":"text/x-python","patch_set":2,"id":"bd5cd979_ebea5beb","line":44,"in_reply_to":"01380a8f_d1df00ba","updated":"2023-02-02 18:03:08.000000000","message":"Good point, will change.","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ffd2f9717e2d20c97f6872403949173904491b27","unresolved":true,"context_lines":[{"line_number":30,"context_line":"        with open(os.path.join(SYS, path), mode\u003d\u0027r\u0027) as data:"},{"line_number":31,"context_line":"            return data.read()"},{"line_number":32,"context_line":"    except (OSError, ValueError):"},{"line_number":33,"context_line":"        raise exception.FileNotFound(file_path\u003dpath)"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"# NOTE(bauzas): this method is deliberately not wrapped in a privsep entrypoint"}],"source_content_type":"text/x-python","patch_set":4,"id":"9f4c2e12_9416479f","line":33,"updated":"2023-02-08 18:01:13.000000000","message":"please use `raise from` (https://legacy.python.org/dev/peps/pep-3134/) so that we can see the original exception content in the stack trace. It will be useful to know if it was a file not found or a permission denied actually.","commit_id":"2d2dedb5a1163e56fa1bf5075f6fd8d85ac77168"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"83e18e2745b72a6e3fa617b83cf14eaf15ef4764","unresolved":false,"context_lines":[{"line_number":30,"context_line":"        with open(os.path.join(SYS, path), mode\u003d\u0027r\u0027) as data:"},{"line_number":31,"context_line":"            return data.read()"},{"line_number":32,"context_line":"    except (OSError, ValueError):"},{"line_number":33,"context_line":"        raise exception.FileNotFound(file_path\u003dpath)"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"# NOTE(bauzas): this method is deliberately not wrapped in a privsep entrypoint"}],"source_content_type":"text/x-python","patch_set":4,"id":"fd0a822e_2d86719a","line":33,"in_reply_to":"9f4c2e12_9416479f","updated":"2023-02-09 05:52:46.000000000","message":"Ack good point","commit_id":"2d2dedb5a1163e56fa1bf5075f6fd8d85ac77168"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ffd2f9717e2d20c97f6872403949173904491b27","unresolved":true,"context_lines":[{"line_number":41,"context_line":"        with open(os.path.join(SYS, path), mode\u003d\u0027w\u0027) as fd:"},{"line_number":42,"context_line":"            fd.write(data)"},{"line_number":43,"context_line":"    except (OSError, ValueError):"},{"line_number":44,"context_line":"        raise exception.FileNotFound(file_path\u003dpath)"}],"source_content_type":"text/x-python","patch_set":4,"id":"740e5b81_c1c7ccee","line":44,"updated":"2023-02-08 18:01:13.000000000","message":"ditto","commit_id":"2d2dedb5a1163e56fa1bf5075f6fd8d85ac77168"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"83e18e2745b72a6e3fa617b83cf14eaf15ef4764","unresolved":false,"context_lines":[{"line_number":41,"context_line":"        with open(os.path.join(SYS, path), mode\u003d\u0027w\u0027) as fd:"},{"line_number":42,"context_line":"            fd.write(data)"},{"line_number":43,"context_line":"    except (OSError, ValueError):"},{"line_number":44,"context_line":"        raise exception.FileNotFound(file_path\u003dpath)"}],"source_content_type":"text/x-python","patch_set":4,"id":"db0a2078_9ecf4afc","line":44,"in_reply_to":"740e5b81_c1c7ccee","updated":"2023-02-09 05:52:46.000000000","message":"Ack","commit_id":"2d2dedb5a1163e56fa1bf5075f6fd8d85ac77168"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d387faf721487153b0d07021c94d53aca64f1fa5","unresolved":false,"context_lines":[{"line_number":28,"context_line":"def read_sys(path: str) -\u003e str:"},{"line_number":29,"context_line":"    \"\"\"Reads the content of a file in the sys filesystem."},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"    :param path: relative or absolute. If relative, will be prefixed by /sys."},{"line_number":32,"context_line":"    :returns: contents of that file."},{"line_number":33,"context_line":"    :raises: nova.exception.FileNotFound if we can\u0027t read that file."},{"line_number":34,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":5,"id":"28e62603_a100970d","line":31,"range":{"start_line":31,"start_character":26,"end_line":31,"end_character":38},"updated":"2023-02-09 17:31:55.000000000","message":"Ohh that is an interesting behavior of os.path.join. Thanks for noteing it","commit_id":"ddf96bcd31bad89ffa391251179ba13bb789991d"}],"nova/tests/unit/test_filesystem.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ffd2f9717e2d20c97f6872403949173904491b27","unresolved":true,"context_lines":[{"line_number":27,"context_line":"        expected_path \u003d os.path.join(filesystem.SYS, \u0027foo\u0027)"},{"line_number":28,"context_line":"        m_open.assert_called_once_with(expected_path, mode\u003d\u0027r\u0027)"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    def test_read_sys_default(self):"},{"line_number":31,"context_line":"        with mock.patch(\u0027builtins.open\u0027,"},{"line_number":32,"context_line":"                        side_effect\u003dOSError(\u0027error\u0027)) as m_open:"},{"line_number":33,"context_line":"            self.assertRaises(exception.FileNotFound,"}],"source_content_type":"text/x-python","patch_set":4,"id":"595ed594_72cbe444","line":30,"range":{"start_line":30,"start_character":22,"end_line":30,"end_character":29},"updated":"2023-02-08 18:01:13.000000000","message":"error","commit_id":"2d2dedb5a1163e56fa1bf5075f6fd8d85ac77168"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"83e18e2745b72a6e3fa617b83cf14eaf15ef4764","unresolved":false,"context_lines":[{"line_number":27,"context_line":"        expected_path \u003d os.path.join(filesystem.SYS, \u0027foo\u0027)"},{"line_number":28,"context_line":"        m_open.assert_called_once_with(expected_path, mode\u003d\u0027r\u0027)"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    def test_read_sys_default(self):"},{"line_number":31,"context_line":"        with mock.patch(\u0027builtins.open\u0027,"},{"line_number":32,"context_line":"                        side_effect\u003dOSError(\u0027error\u0027)) as m_open:"},{"line_number":33,"context_line":"            self.assertRaises(exception.FileNotFound,"}],"source_content_type":"text/x-python","patch_set":4,"id":"37a3ddf0_10688a04","line":30,"range":{"start_line":30,"start_character":22,"end_line":30,"end_character":29},"in_reply_to":"595ed594_72cbe444","updated":"2023-02-09 05:52:46.000000000","message":"Ack","commit_id":"2d2dedb5a1163e56fa1bf5075f6fd8d85ac77168"}],"nova/tests/unit/virt/libvirt/cpu/test_api.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ffd2f9717e2d20c97f6872403949173904491b27","unresolved":true,"context_lines":[{"line_number":51,"context_line":"    @mock.patch.object(core, \u0027set_governor\u0027)"},{"line_number":52,"context_line":"    def test_set_governor_low(self, mock_set_governor):"},{"line_number":53,"context_line":"        self.flags(cpu_power_governor_low\u003d\u0027fake_low_gov\u0027, group\u003d\u0027libvirt\u0027)"},{"line_number":54,"context_line":"        self.assertIsNone(self.core_1.set_low_governor())"},{"line_number":55,"context_line":"        mock_set_governor.assert_called_once_with(self.core_1.ident,"},{"line_number":56,"context_line":"                                                  \u0027fake_low_gov\u0027)"},{"line_number":57,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"c2053923_43cf262e","line":54,"updated":"2023-02-08 18:01:13.000000000","message":"this is not a real assert as set_low_governor always return None","commit_id":"2d2dedb5a1163e56fa1bf5075f6fd8d85ac77168"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"83e18e2745b72a6e3fa617b83cf14eaf15ef4764","unresolved":false,"context_lines":[{"line_number":51,"context_line":"    @mock.patch.object(core, \u0027set_governor\u0027)"},{"line_number":52,"context_line":"    def test_set_governor_low(self, mock_set_governor):"},{"line_number":53,"context_line":"        self.flags(cpu_power_governor_low\u003d\u0027fake_low_gov\u0027, group\u003d\u0027libvirt\u0027)"},{"line_number":54,"context_line":"        self.assertIsNone(self.core_1.set_low_governor())"},{"line_number":55,"context_line":"        mock_set_governor.assert_called_once_with(self.core_1.ident,"},{"line_number":56,"context_line":"                                                  \u0027fake_low_gov\u0027)"},{"line_number":57,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"00945cb7_39ede885","line":54,"in_reply_to":"c2053923_43cf262e","updated":"2023-02-09 05:52:46.000000000","message":"Ack","commit_id":"2d2dedb5a1163e56fa1bf5075f6fd8d85ac77168"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ffd2f9717e2d20c97f6872403949173904491b27","unresolved":true,"context_lines":[{"line_number":58,"context_line":"    @mock.patch.object(core, \u0027set_governor\u0027)"},{"line_number":59,"context_line":"    def test_set_governor_high(self, mock_set_governor):"},{"line_number":60,"context_line":"        self.flags(cpu_power_governor_high\u003d\u0027fake_high_gov\u0027, group\u003d\u0027libvirt\u0027)"},{"line_number":61,"context_line":"        self.assertIsNone(self.core_1.set_high_governor())"},{"line_number":62,"context_line":"        mock_set_governor.assert_called_once_with(self.core_1.ident,"},{"line_number":63,"context_line":"                                                  \u0027fake_high_gov\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3cf3910c_e256316c","line":61,"updated":"2023-02-08 18:01:13.000000000","message":"ditto","commit_id":"2d2dedb5a1163e56fa1bf5075f6fd8d85ac77168"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"83e18e2745b72a6e3fa617b83cf14eaf15ef4764","unresolved":false,"context_lines":[{"line_number":58,"context_line":"    @mock.patch.object(core, \u0027set_governor\u0027)"},{"line_number":59,"context_line":"    def test_set_governor_high(self, mock_set_governor):"},{"line_number":60,"context_line":"        self.flags(cpu_power_governor_high\u003d\u0027fake_high_gov\u0027, group\u003d\u0027libvirt\u0027)"},{"line_number":61,"context_line":"        self.assertIsNone(self.core_1.set_high_governor())"},{"line_number":62,"context_line":"        mock_set_governor.assert_called_once_with(self.core_1.ident,"},{"line_number":63,"context_line":"                                                  \u0027fake_high_gov\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"56727dd8_510a2538","line":61,"in_reply_to":"3cf3910c_e256316c","updated":"2023-02-09 05:52:46.000000000","message":"Ack","commit_id":"2d2dedb5a1163e56fa1bf5075f6fd8d85ac77168"}],"nova/virt/libvirt/cpu/__init__.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a2700a057aa2f3845e5739e4fc7226e0feb005bf","unresolved":true,"context_lines":[{"line_number":22,"context_line":"CONF \u003d nova.conf.CONF"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"GOVERNOR_HIGH \u003d \u0027high\u0027"},{"line_number":25,"context_line":"GOVERNOR_LOW \u003d \u0027now\u0027"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"@dataclass"}],"source_content_type":"text/x-python","patch_set":2,"id":"7c8fa763_5b14dff8","line":25,"range":{"start_line":25,"start_character":16,"end_line":25,"end_character":19},"updated":"2023-02-01 18:56:18.000000000","message":"low?","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"4d362177b7e1c11f6469312e0ef1b81d6a6ce0be","unresolved":false,"context_lines":[{"line_number":22,"context_line":"CONF \u003d nova.conf.CONF"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"GOVERNOR_HIGH \u003d \u0027high\u0027"},{"line_number":25,"context_line":"GOVERNOR_LOW \u003d \u0027now\u0027"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"@dataclass"}],"source_content_type":"text/x-python","patch_set":2,"id":"b4998876_87469db0","line":25,"range":{"start_line":25,"start_character":16,"end_line":25,"end_character":19},"in_reply_to":"7c8fa763_5b14dff8","updated":"2023-02-02 18:03:08.000000000","message":"Ack","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a2700a057aa2f3845e5739e4fc7226e0feb005bf","unresolved":true,"context_lines":[{"line_number":40,"context_line":""},{"line_number":41,"context_line":"    @property"},{"line_number":42,"context_line":"    def path(self) -\u003e str:"},{"line_number":43,"context_line":"        if self._path is None:"},{"line_number":44,"context_line":"            self._path \u003d core.gen_cpu_path(self.ident)"},{"line_number":45,"context_line":"        return self._path"},{"line_number":46,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"10befce0_732c2cf3","line":43,"updated":"2023-02-01 18:56:18.000000000","message":"This can lead to inconsistencies\n```\n\u003e\u003e\u003e from nova.virt.libvirt import cpu\n\u003e\u003e\u003e c \u003d cpu.Core(2)\n\u003e\u003e\u003e c\nCore(ident\u003d2, _path\u003dNone)\n\u003e\u003e\u003e c.path\n\u0027/sys/devices/system/cpu/cpu2\u0027\n\u003e\u003e\u003e c.ident\u003d3\n\u003e\u003e\u003e c.path\n\u0027/sys/devices/system/cpu/cpu2\u0027\n\u003e\u003e\u003e\n```\n\nSo\na) if path is cheap to generate then lets always generate it based on ident\nb) if path expensive to generate then hide ident to `_ident` and add a getter property for it to signal it is not meant to be changed.\nc) if path is expensive and you still want to allow ident to change then create an ident setter and generate the path once in the ctor and once in the ident setter.","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"78cd995a0d82d14b832407978e342343ecdfd04e","unresolved":false,"context_lines":[{"line_number":40,"context_line":""},{"line_number":41,"context_line":"    @property"},{"line_number":42,"context_line":"    def path(self) -\u003e str:"},{"line_number":43,"context_line":"        if self._path is None:"},{"line_number":44,"context_line":"            self._path \u003d core.gen_cpu_path(self.ident)"},{"line_number":45,"context_line":"        return self._path"},{"line_number":46,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9625f70f_79e37e25","line":43,"in_reply_to":"10befce0_732c2cf3","updated":"2023-02-01 19:01:19.000000000","message":"OK, in PS3 you opt into option a) and pushed the path down the stack. that works for me.","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a2700a057aa2f3845e5739e4fc7226e0feb005bf","unresolved":true,"context_lines":[{"line_number":55,"context_line":"        else:"},{"line_number":56,"context_line":"            core.set_offline(self.path)"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    def __hash__(self):"},{"line_number":59,"context_line":"        return hash(self.ident)"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    @property"}],"source_content_type":"text/x-python","patch_set":2,"id":"e077ef4b_f7c0ecf6","line":58,"updated":"2023-02-01 18:56:18.000000000","message":"while it is not mandatory I would add __eq__ too to define for the next developer that the intention is that two Core equals if the ident equals regardless of path","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"4d362177b7e1c11f6469312e0ef1b81d6a6ce0be","unresolved":false,"context_lines":[{"line_number":55,"context_line":"        else:"},{"line_number":56,"context_line":"            core.set_offline(self.path)"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    def __hash__(self):"},{"line_number":59,"context_line":"        return hash(self.ident)"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    @property"}],"source_content_type":"text/x-python","patch_set":2,"id":"c247f286_16c1a481","line":58,"in_reply_to":"e077ef4b_f7c0ecf6","updated":"2023-02-02 18:03:08.000000000","message":"Ack","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a2700a057aa2f3845e5739e4fc7226e0feb005bf","unresolved":true,"context_lines":[{"line_number":62,"context_line":"    def governor(self) -\u003e str:"},{"line_number":63,"context_line":"        return core.get_governor(self.path)"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    @governor.setter"},{"line_number":66,"context_line":"    def governor(self, level: str) -\u003e None:"},{"line_number":67,"context_line":"        if level not in (GOVERNOR_HIGH, GOVERNOR_LOW):"},{"line_number":68,"context_line":"            LOG.warning(\"Invalid value for a CPU governor: %s\", level)"},{"line_number":69,"context_line":"            return"},{"line_number":70,"context_line":"        if level \u003d\u003d GOVERNOR_HIGH:"},{"line_number":71,"context_line":"            core.set_governor(self.path, CONF.libvirt.cpu_power_governor_high)"},{"line_number":72,"context_line":"        else:"},{"line_number":73,"context_line":"            core.set_governor(self.path, CONF.libvirt.cpu_power_governor_low)"}],"source_content_type":"text/x-python","patch_set":2,"id":"e79a8c82_f75b6910","line":73,"range":{"start_line":65,"start_character":0,"end_line":73,"end_character":77},"updated":"2023-02-01 18:56:18.000000000","message":"if you anyhow restricts the values that can be passed to governor to two possible values then I think the interface is more readable as:\n```\ndef set_high_governor()\n\ndef set_low_governor()\n```\nThis also removes the need to handle that an unexpected value passed in. I.e. the interface already prevents the wrong value.","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"4d362177b7e1c11f6469312e0ef1b81d6a6ce0be","unresolved":false,"context_lines":[{"line_number":62,"context_line":"    def governor(self) -\u003e str:"},{"line_number":63,"context_line":"        return core.get_governor(self.path)"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    @governor.setter"},{"line_number":66,"context_line":"    def governor(self, level: str) -\u003e None:"},{"line_number":67,"context_line":"        if level not in (GOVERNOR_HIGH, GOVERNOR_LOW):"},{"line_number":68,"context_line":"            LOG.warning(\"Invalid value for a CPU governor: %s\", level)"},{"line_number":69,"context_line":"            return"},{"line_number":70,"context_line":"        if level \u003d\u003d GOVERNOR_HIGH:"},{"line_number":71,"context_line":"            core.set_governor(self.path, CONF.libvirt.cpu_power_governor_high)"},{"line_number":72,"context_line":"        else:"},{"line_number":73,"context_line":"            core.set_governor(self.path, CONF.libvirt.cpu_power_governor_low)"}],"source_content_type":"text/x-python","patch_set":2,"id":"28e7781f_806f5924","line":73,"range":{"start_line":65,"start_character":0,"end_line":73,"end_character":77},"in_reply_to":"e79a8c82_f75b6910","updated":"2023-02-02 18:03:08.000000000","message":"Ack","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"}],"nova/virt/libvirt/cpu/api.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"78cd995a0d82d14b832407978e342343ecdfd04e","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"eaadc0f3_d63a6ce6","updated":"2023-02-01 19:01:19.000000000","message":"please seem my comments from PS2 where it was in __init__.py The unresolved ones seems to be valid for me in PS3 too","commit_id":"021cc6408c611cce5422398f3c16c070aa78a334"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"4d362177b7e1c11f6469312e0ef1b81d6a6ce0be","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"967f9365_3803dd23","in_reply_to":"eaadc0f3_d63a6ce6","updated":"2023-02-02 18:03:08.000000000","message":"Ack","commit_id":"021cc6408c611cce5422398f3c16c070aa78a334"}],"nova/virt/libvirt/cpu/core.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a2700a057aa2f3845e5739e4fc7226e0feb005bf","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"a1199fd6_89f52df7","updated":"2023-02-01 18:56:18.000000000","message":"This is pretty common thing I fairly certain somebody already created a lib for it.\n\n// later\n\nThis seem to fit the bill https://github.com/VitorRamos/cpufreq","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ffd2f9717e2d20c97f6872403949173904491b27","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"8312487c_8104a0d8","in_reply_to":"2d228c5d_063b4c6c","updated":"2023-02-08 18:01:13.000000000","message":"meh. :)","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"4d362177b7e1c11f6469312e0ef1b81d6a6ce0be","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"2d228c5d_063b4c6c","in_reply_to":"a1199fd6_89f52df7","updated":"2023-02-02 18:03:08.000000000","message":"Well, we already directly call sysfs on some other methods and I don\u0027t like to have another dependency for Nova to usesome GH project.","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"086921af1b2b8caf65d1c64860283b38e1c33702","unresolved":true,"context_lines":[{"line_number":22,"context_line":""},{"line_number":23,"context_line":"AVAILABLE_PATH \u003d \u0027devices/system/cpu/present\u0027"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"CPU_PATH_TEMPLATE \u003d \u0027devices/system/cpu/cpu\u0027"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"def get_available_cores() -\u003e ty.Set[int]:"}],"source_content_type":"text/x-python","patch_set":2,"id":"beec2263_de8f4550","line":25,"updated":"2023-01-24 18:11:30.000000000","message":"I would have used a fstring for the template so you don\u0027t have to cast manually.\n\n`CPU_PATH_TEMPLATE\u003d\u0027/devices/../../cpu/%(cpuid)s\u0027`","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"19af0bd40142aa37e7fc27441f629c7739aa6213","unresolved":false,"context_lines":[{"line_number":22,"context_line":""},{"line_number":23,"context_line":"AVAILABLE_PATH \u003d \u0027devices/system/cpu/present\u0027"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"CPU_PATH_TEMPLATE \u003d \u0027devices/system/cpu/cpu\u0027"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"def get_available_cores() -\u003e ty.Set[int]:"}],"source_content_type":"text/x-python","patch_set":2,"id":"128a859b_5639695e","line":25,"in_reply_to":"beec2263_de8f4550","updated":"2023-01-31 19:11:03.000000000","message":"Well, technically, it can\u0027t be a f-string since the cpuid isn\u0027t known when we instantiate the string, so it would only be a pattern like you wrote but surely I can propose an alternative.\n\nDone.","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"086921af1b2b8caf65d1c64860283b38e1c33702","unresolved":true,"context_lines":[{"line_number":23,"context_line":"AVAILABLE_PATH \u003d \u0027devices/system/cpu/present\u0027"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"CPU_PATH_TEMPLATE \u003d \u0027devices/system/cpu/cpu\u0027"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"def get_available_cores() -\u003e ty.Set[int]:"},{"line_number":29,"context_line":"    return hardware.parse_cpu_spec("}],"source_content_type":"text/x-python","patch_set":2,"id":"f2dca57a_66708d9f","line":26,"updated":"2023-01-24 18:11:30.000000000","message":"Perhaps we could have here enum of the valid governors, the function should probably have some validation steps of the accepted values, don\u0027t you think? \n\nThey will then raise some specific exceptions, the upper layer could catch them to expose error to the users.","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"467cf0586b217ec18cda0dbbaecf6aa6496c7c6e","unresolved":true,"context_lines":[{"line_number":23,"context_line":"AVAILABLE_PATH \u003d \u0027devices/system/cpu/present\u0027"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"CPU_PATH_TEMPLATE \u003d \u0027devices/system/cpu/cpu\u0027"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"def get_available_cores() -\u003e ty.Set[int]:"},{"line_number":29,"context_line":"    return hardware.parse_cpu_spec("}],"source_content_type":"text/x-python","patch_set":2,"id":"e4ccc421_07308dbd","line":26,"in_reply_to":"c94d19da_9997a094","updated":"2023-02-10 08:58:56.000000000","message":"we could technicly get the available govenors from sysfs and assert the config values are a subset for that but we should not have a static list.\nwe could add this as a followup change but its not required for this feature.","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"19af0bd40142aa37e7fc27441f629c7739aa6213","unresolved":true,"context_lines":[{"line_number":23,"context_line":"AVAILABLE_PATH \u003d \u0027devices/system/cpu/present\u0027"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"CPU_PATH_TEMPLATE \u003d \u0027devices/system/cpu/cpu\u0027"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"def get_available_cores() -\u003e ty.Set[int]:"},{"line_number":29,"context_line":"    return hardware.parse_cpu_spec("}],"source_content_type":"text/x-python","patch_set":2,"id":"c94d19da_9997a094","line":26,"in_reply_to":"f2dca57a_66708d9f","updated":"2023-01-31 19:11:03.000000000","message":"As we said in the spec, we can\u0027t really have enums, but if you look at the last patch of the series, we do some kind of limited input validation indeed.","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"086921af1b2b8caf65d1c64860283b38e1c33702","unresolved":true,"context_lines":[{"line_number":37,"context_line":""},{"line_number":38,"context_line":"def gen_cpu_path(core: int) -\u003e str:"},{"line_number":39,"context_line":"    if not exists(core):"},{"line_number":40,"context_line":"        err \u003d ValueError(\u0027CPU: %(core)s does not exist\u0027, core)"},{"line_number":41,"context_line":"        LOG.warning(\u0027Unable to access CPU: %s\u0027, core)"},{"line_number":42,"context_line":"        raise err"},{"line_number":43,"context_line":"    sys \u003d filesystem.SYS"}],"source_content_type":"text/x-python","patch_set":2,"id":"64007e6f_d52eedd7","line":40,"updated":"2023-01-24 18:11:30.000000000","message":"The `err` variable looks not necessary.","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"19af0bd40142aa37e7fc27441f629c7739aa6213","unresolved":false,"context_lines":[{"line_number":37,"context_line":""},{"line_number":38,"context_line":"def gen_cpu_path(core: int) -\u003e str:"},{"line_number":39,"context_line":"    if not exists(core):"},{"line_number":40,"context_line":"        err \u003d ValueError(\u0027CPU: %(core)s does not exist\u0027, core)"},{"line_number":41,"context_line":"        LOG.warning(\u0027Unable to access CPU: %s\u0027, core)"},{"line_number":42,"context_line":"        raise err"},{"line_number":43,"context_line":"    sys \u003d filesystem.SYS"}],"source_content_type":"text/x-python","patch_set":2,"id":"eaac3d8d_787ee328","line":40,"in_reply_to":"64007e6f_d52eedd7","updated":"2023-01-31 19:11:03.000000000","message":"I can optimize.","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a2700a057aa2f3845e5739e4fc7226e0feb005bf","unresolved":true,"context_lines":[{"line_number":40,"context_line":"        err \u003d ValueError(\u0027CPU: %(core)s does not exist\u0027, core)"},{"line_number":41,"context_line":"        LOG.warning(\u0027Unable to access CPU: %s\u0027, core)"},{"line_number":42,"context_line":"        raise err"},{"line_number":43,"context_line":"    sys \u003d filesystem.SYS"},{"line_number":44,"context_line":"    return str(os.path.join(sys, CPU_PATH_TEMPLATE + str(core)))"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"caff2a61_f25d384e","line":43,"updated":"2023-02-01 18:56:18.000000000","message":"It feels strange that SYS is special and in filesystems while the rest of the path is defined here. I think it is an unnecessary separation. AVAILABLE_PATH is alway under sys. So I would simply define AVAILABLE_PATH \u003d /sys/devices/system/cpu/present","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ffd2f9717e2d20c97f6872403949173904491b27","unresolved":true,"context_lines":[{"line_number":40,"context_line":"        err \u003d ValueError(\u0027CPU: %(core)s does not exist\u0027, core)"},{"line_number":41,"context_line":"        LOG.warning(\u0027Unable to access CPU: %s\u0027, core)"},{"line_number":42,"context_line":"        raise err"},{"line_number":43,"context_line":"    sys \u003d filesystem.SYS"},{"line_number":44,"context_line":"    return str(os.path.join(sys, CPU_PATH_TEMPLATE + str(core)))"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1582cc31_51374440","line":43,"in_reply_to":"0632165f_46158f56","updated":"2023-02-08 18:01:13.000000000","message":"I would rather keep the whole path in a single place (here) as /sys/devices/system/cpu/present and change the mock to manipulate these constants to fit the test.","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"83e18e2745b72a6e3fa617b83cf14eaf15ef4764","unresolved":false,"context_lines":[{"line_number":40,"context_line":"        err \u003d ValueError(\u0027CPU: %(core)s does not exist\u0027, core)"},{"line_number":41,"context_line":"        LOG.warning(\u0027Unable to access CPU: %s\u0027, core)"},{"line_number":42,"context_line":"        raise err"},{"line_number":43,"context_line":"    sys \u003d filesystem.SYS"},{"line_number":44,"context_line":"    return str(os.path.join(sys, CPU_PATH_TEMPLATE + str(core)))"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"664f8075_4d956730","line":43,"in_reply_to":"1582cc31_51374440","updated":"2023-02-09 05:52:46.000000000","message":"Ack","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"4d362177b7e1c11f6469312e0ef1b81d6a6ce0be","unresolved":true,"context_lines":[{"line_number":40,"context_line":"        err \u003d ValueError(\u0027CPU: %(core)s does not exist\u0027, core)"},{"line_number":41,"context_line":"        LOG.warning(\u0027Unable to access CPU: %s\u0027, core)"},{"line_number":42,"context_line":"        raise err"},{"line_number":43,"context_line":"    sys \u003d filesystem.SYS"},{"line_number":44,"context_line":"    return str(os.path.join(sys, CPU_PATH_TEMPLATE + str(core)))"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"0632165f_46158f56","line":43,"in_reply_to":"caff2a61_f25d384e","updated":"2023-02-02 18:03:08.000000000","message":"This is mostly due because we mock the SYS variable in the functest here : \nhttps://review.opendev.org/c/openstack/nova/+/868237/5/nova/tests/functional/libvirt/test_power_manage.py#61","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"086921af1b2b8caf65d1c64860283b38e1c33702","unresolved":true,"context_lines":[{"line_number":41,"context_line":"        LOG.warning(\u0027Unable to access CPU: %s\u0027, core)"},{"line_number":42,"context_line":"        raise err"},{"line_number":43,"context_line":"    sys \u003d filesystem.SYS"},{"line_number":44,"context_line":"    return str(os.path.join(sys, CPU_PATH_TEMPLATE + str(core)))"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"def get_online(cpu_path: str) -\u003e bool:"}],"source_content_type":"text/x-python","patch_set":2,"id":"932e6f7b_208246c5","line":44,"updated":"2023-01-24 18:11:30.000000000","message":"I suggested something about using fstring template that could avoid the `str()`","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"19af0bd40142aa37e7fc27441f629c7739aa6213","unresolved":false,"context_lines":[{"line_number":41,"context_line":"        LOG.warning(\u0027Unable to access CPU: %s\u0027, core)"},{"line_number":42,"context_line":"        raise err"},{"line_number":43,"context_line":"    sys \u003d filesystem.SYS"},{"line_number":44,"context_line":"    return str(os.path.join(sys, CPU_PATH_TEMPLATE + str(core)))"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"def get_online(cpu_path: str) -\u003e bool:"}],"source_content_type":"text/x-python","patch_set":2,"id":"60a40ddc_3766af5e","line":44,"in_reply_to":"932e6f7b_208246c5","updated":"2023-01-31 19:11:03.000000000","message":"Ack","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"57031804d97230c6df4554f76d0781091a926e33","unresolved":true,"context_lines":[{"line_number":44,"context_line":"    return str(os.path.join(sys, CPU_PATH_TEMPLATE + str(core)))"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"def get_online(cpu_path: str) -\u003e bool:"},{"line_number":48,"context_line":"    online \u003d filesystem.read_sys("},{"line_number":49,"context_line":"        os.path.join(cpu_path, \u0027online\u0027), default\u003d\u00271\u0027).strip()"},{"line_number":50,"context_line":"    return online \u003d\u003d \u00271\u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"5e519648_d48ac655","line":47,"updated":"2023-01-24 00:19:47.000000000","message":"ok i no this is an artifact of the arbiterd implemenation but this should take the core as an int and gerneate teh path internally","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"086921af1b2b8caf65d1c64860283b38e1c33702","unresolved":true,"context_lines":[{"line_number":44,"context_line":"    return str(os.path.join(sys, CPU_PATH_TEMPLATE + str(core)))"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"def get_online(cpu_path: str) -\u003e bool:"},{"line_number":48,"context_line":"    online \u003d filesystem.read_sys("},{"line_number":49,"context_line":"        os.path.join(cpu_path, \u0027online\u0027), default\u003d\u00271\u0027).strip()"},{"line_number":50,"context_line":"    return online \u003d\u003d \u00271\u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"e67f5123_ed6735e1","line":47,"in_reply_to":"5e519648_d48ac655","updated":"2023-01-24 18:11:30.000000000","message":"I\u0027m kind of agree, you want to pass ID of the core that you want to make online","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"385c0638dfd1fd14465a0e707a1154e42e9d4282","unresolved":true,"context_lines":[{"line_number":44,"context_line":"    return str(os.path.join(sys, CPU_PATH_TEMPLATE + str(core)))"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"def get_online(cpu_path: str) -\u003e bool:"},{"line_number":48,"context_line":"    online \u003d filesystem.read_sys("},{"line_number":49,"context_line":"        os.path.join(cpu_path, \u0027online\u0027), default\u003d\u00271\u0027).strip()"},{"line_number":50,"context_line":"    return online \u003d\u003d \u00271\u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"fc7bd5df_e1f7112a","line":47,"in_reply_to":"e67f5123_ed6735e1","updated":"2023-01-30 16:09:04.000000000","message":"Maybe you haven\u0027t seen it but actually the API only have the core to be an integer here https://review.opendev.org/c/openstack/nova/+/868236/2/nova/virt/libvirt/cpu/__init__.py\n\nThat being said, I can just create the Core API here and directly call filesystem in the methods API methods.","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"19af0bd40142aa37e7fc27441f629c7739aa6213","unresolved":false,"context_lines":[{"line_number":44,"context_line":"    return str(os.path.join(sys, CPU_PATH_TEMPLATE + str(core)))"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"def get_online(cpu_path: str) -\u003e bool:"},{"line_number":48,"context_line":"    online \u003d filesystem.read_sys("},{"line_number":49,"context_line":"        os.path.join(cpu_path, \u0027online\u0027), default\u003d\u00271\u0027).strip()"},{"line_number":50,"context_line":"    return online \u003d\u003d \u00271\u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"238b3db5_ae77c194","line":47,"in_reply_to":"fc7bd5df_e1f7112a","updated":"2023-01-31 19:11:03.000000000","message":"Ack","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"57031804d97230c6df4554f76d0781091a926e33","unresolved":true,"context_lines":[{"line_number":50,"context_line":"    return online \u003d\u003d \u00271\u0027"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"def set_online(cpu_path: str) -\u003e bool:"},{"line_number":54,"context_line":"    online \u003d get_online(cpu_path)"},{"line_number":55,"context_line":"    if online:"},{"line_number":56,"context_line":"        return True"}],"source_content_type":"text/x-python","patch_set":2,"id":"09c55852_e45d51b5","line":53,"updated":"2023-01-24 00:19:47.000000000","message":"this method should also take the core id not a path and this should have a prevsep decorator\n\nthe filestem.write_sys funciton should not be prviadted as i said earlier\n\nthe contract of the function is two broad.\n\nbut the contract fo set_online is narrow, its just an int so its safer to decorate this funciton.","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"19af0bd40142aa37e7fc27441f629c7739aa6213","unresolved":false,"context_lines":[{"line_number":50,"context_line":"    return online \u003d\u003d \u00271\u0027"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"def set_online(cpu_path: str) -\u003e bool:"},{"line_number":54,"context_line":"    online \u003d get_online(cpu_path)"},{"line_number":55,"context_line":"    if online:"},{"line_number":56,"context_line":"        return True"}],"source_content_type":"text/x-python","patch_set":2,"id":"54d0fca9_92549c4f","line":53,"in_reply_to":"09c55852_e45d51b5","updated":"2023-01-31 19:11:03.000000000","message":"Ack","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"57031804d97230c6df4554f76d0781091a926e33","unresolved":true,"context_lines":[{"line_number":58,"context_line":"    return get_online(cpu_path)"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"def set_offline(cpu_path: str) -\u003e bool:"},{"line_number":62,"context_line":"    online \u003d get_online(cpu_path)"},{"line_number":63,"context_line":"    if not online:"},{"line_number":64,"context_line":"        return True"}],"source_content_type":"text/x-python","patch_set":2,"id":"a2a6f722_1457b601","line":61,"updated":"2023-01-24 00:19:47.000000000","message":"same here and set_governor\n\nthese should have cpu ids as inputs and be decorated as prvaldged","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"19af0bd40142aa37e7fc27441f629c7739aa6213","unresolved":false,"context_lines":[{"line_number":58,"context_line":"    return get_online(cpu_path)"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"def set_offline(cpu_path: str) -\u003e bool:"},{"line_number":62,"context_line":"    online \u003d get_online(cpu_path)"},{"line_number":63,"context_line":"    if not online:"},{"line_number":64,"context_line":"        return True"}],"source_content_type":"text/x-python","patch_set":2,"id":"92b0dcf1_9f037491","line":61,"in_reply_to":"a2a6f722_1457b601","updated":"2023-01-31 19:11:03.000000000","message":"Ack","commit_id":"548ff5dee98ef4574f3801acb91f7696f23130d8"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"41f6d9580d044602361d8bd6c0fd9390ed798fcf","unresolved":true,"context_lines":[{"line_number":52,"context_line":""},{"line_number":53,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"},{"line_number":54,"context_line":"def set_online(core: int) -\u003e bool:"},{"line_number":55,"context_line":"    online \u003d get_online(core)"},{"line_number":56,"context_line":"    if online:"},{"line_number":57,"context_line":"        return True"},{"line_number":58,"context_line":"    filesystem.write_sys(os.path.join(gen_cpu_path(core), \u0027online\u0027), data\u003d\u00271\u0027)"},{"line_number":59,"context_line":"    return get_online(core)"},{"line_number":60,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"845619c1_767a9740","line":57,"range":{"start_line":55,"start_character":0,"end_line":57,"end_character":19},"updated":"2023-02-01 19:11:26.000000000","message":"I don\u0027t think this write optimization worth the extra logic. I think we are free to write to sys as frequently as we wish, this is not a disk IO.","commit_id":"021cc6408c611cce5422398f3c16c070aa78a334"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"fd02d02cfaad0b7b5505f49b7ffe8e69fed3505a","unresolved":false,"context_lines":[{"line_number":52,"context_line":""},{"line_number":53,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"},{"line_number":54,"context_line":"def set_online(core: int) -\u003e bool:"},{"line_number":55,"context_line":"    online \u003d get_online(core)"},{"line_number":56,"context_line":"    if online:"},{"line_number":57,"context_line":"        return True"},{"line_number":58,"context_line":"    filesystem.write_sys(os.path.join(gen_cpu_path(core), \u0027online\u0027), data\u003d\u00271\u0027)"},{"line_number":59,"context_line":"    return get_online(core)"},{"line_number":60,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1f37e392_61aa72e2","line":57,"range":{"start_line":55,"start_character":0,"end_line":57,"end_character":19},"in_reply_to":"845619c1_767a9740","updated":"2023-02-02 20:39:33.000000000","message":"Ack","commit_id":"021cc6408c611cce5422398f3c16c070aa78a334"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"41f6d9580d044602361d8bd6c0fd9390ed798fcf","unresolved":true,"context_lines":[{"line_number":60,"context_line":""},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"def set_offline(core: int) -\u003e bool:"},{"line_number":63,"context_line":"    online \u003d get_online(core)"},{"line_number":64,"context_line":"    if not online:"},{"line_number":65,"context_line":"        return True"},{"line_number":66,"context_line":"    filesystem.write_sys(os.path.join(gen_cpu_path(core), \u0027online\u0027), data\u003d\u00270\u0027)"},{"line_number":67,"context_line":"    return not get_online(core)"},{"line_number":68,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"0cef010b_618c2210","line":65,"range":{"start_line":63,"start_character":1,"end_line":65,"end_character":19},"updated":"2023-02-01 19:11:26.000000000","message":"ditto","commit_id":"021cc6408c611cce5422398f3c16c070aa78a334"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"fd02d02cfaad0b7b5505f49b7ffe8e69fed3505a","unresolved":false,"context_lines":[{"line_number":60,"context_line":""},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"def set_offline(core: int) -\u003e bool:"},{"line_number":63,"context_line":"    online \u003d get_online(core)"},{"line_number":64,"context_line":"    if not online:"},{"line_number":65,"context_line":"        return True"},{"line_number":66,"context_line":"    filesystem.write_sys(os.path.join(gen_cpu_path(core), \u0027online\u0027), data\u003d\u00270\u0027)"},{"line_number":67,"context_line":"    return not get_online(core)"},{"line_number":68,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"c4d78cb2_77212bd1","line":65,"range":{"start_line":63,"start_character":1,"end_line":65,"end_character":19},"in_reply_to":"0cef010b_618c2210","updated":"2023-02-02 20:39:33.000000000","message":"Ack","commit_id":"021cc6408c611cce5422398f3c16c070aa78a334"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"41f6d9580d044602361d8bd6c0fd9390ed798fcf","unresolved":true,"context_lines":[{"line_number":74,"context_line":""},{"line_number":75,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"},{"line_number":76,"context_line":"def set_governor(core: int, governor: str) -\u003e str:"},{"line_number":77,"context_line":"    filesystem.write_sys(os.path.join(gen_cpu_path(core),"},{"line_number":78,"context_line":"                                      \u0027cpufreq/scaling_governor\u0027),"},{"line_number":79,"context_line":"                         data\u003dgovernor)"},{"line_number":80,"context_line":"    return get_governor(core)"}],"source_content_type":"text/x-python","patch_set":3,"id":"5ad5ecc7_d966f765","line":79,"range":{"start_line":77,"start_character":0,"end_line":79,"end_character":39},"updated":"2023-02-01 19:11:26.000000000","message":"I would format this:\n\n```\n    filesystem.write_sys(\n        os.path.join(gen_cpu_path(core),\u0027cpufreq/scaling_governor\u0027),\n        data\u003dgovernor\n    )\n```","commit_id":"021cc6408c611cce5422398f3c16c070aa78a334"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"fd02d02cfaad0b7b5505f49b7ffe8e69fed3505a","unresolved":false,"context_lines":[{"line_number":74,"context_line":""},{"line_number":75,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"},{"line_number":76,"context_line":"def set_governor(core: int, governor: str) -\u003e str:"},{"line_number":77,"context_line":"    filesystem.write_sys(os.path.join(gen_cpu_path(core),"},{"line_number":78,"context_line":"                                      \u0027cpufreq/scaling_governor\u0027),"},{"line_number":79,"context_line":"                         data\u003dgovernor)"},{"line_number":80,"context_line":"    return get_governor(core)"}],"source_content_type":"text/x-python","patch_set":3,"id":"8af0d012_c62cab5f","line":79,"range":{"start_line":77,"start_character":0,"end_line":79,"end_character":39},"in_reply_to":"5ad5ecc7_d966f765","updated":"2023-02-02 20:39:33.000000000","message":"Ack","commit_id":"021cc6408c611cce5422398f3c16c070aa78a334"}]}