)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":33594,"name":"Thales Elero Cervi","display_name":"Thales Cervi","email":"thaleselero.cervi@windriver.com","username":"tcervi"},"change_message_id":"43020ed6344c40fa757e4779e034923bf7a1162f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"01957410_064c5503","updated":"2021-12-22 12:37:13.000000000","message":"LGTM","commit_id":"9f9c25f4f9b8c4ec3cadc7f5cae2a87ec1812aef"},{"author":{"_account_id":15435,"name":"Al Bailey","email":"albailey1974@gmail.com","username":"albailey"},"change_message_id":"f2b31ae25778c52d3149896582ea9ef4621f5b97","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"1650a4be_8d9287a2","updated":"2021-12-27 22:31:31.000000000","message":"seems fine to me","commit_id":"a669f5bcf664c31a3a851e51bfd9eaa12cd33f16"},{"author":{"_account_id":32349,"name":"Thiago Paiva Brito","email":"thiago.brito@windriver.com","username":"outbrito"},"change_message_id":"5336e7b20dab7d943f7ec282853363e3ac18407c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"064ed12a_c16c0955","updated":"2022-01-05 12:44:06.000000000","message":"I think we should strive to use oslo.config as is was meant to be used instead of using configparser (since oslo.config has some useful stuff like checking the parameter types). You should pass the --config-file at service start and you would also be able to remove the whole [parameters] section and the StrOpt \"config_file\", making the code a little simpler.\n\nPlease, check if we will still use this service on-metal after this change. If so, we will need to pass the --config-file argument also on the systemctl script that starts it.\n\nFor reference: \nhttps://docs.openstack.org/oslo.config/latest/configuration/quickstart.html\nhttps://docs.openstack.org/oslo.config/latest/configuration/options.html (defaults)\n","commit_id":"293388bca562631652ca1db6a85beff5e5423192"},{"author":{"_account_id":33390,"name":"Heitor Matsui","display_name":"Heitor Matsui","email":"HeitorVieira.Matsui@windriver.com","username":"heitormatsui"},"change_message_id":"76a53ccadaf58ed17626f97a526f07df7c013853","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"0a8466df_67123997","in_reply_to":"064ed12a_c16c0955","updated":"2022-01-06 15:52:18.000000000","message":"Done","commit_id":"293388bca562631652ca1db6a85beff5e5423192"},{"author":{"_account_id":14675,"name":"Hugo Nicodemos","email":"hugo.brito@windriver.com","username":"nicodemos"},"change_message_id":"1a7e75f76a0ddb54b0c835f776c1f2f5d3ff22ad","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"c97d9428_40097f51","updated":"2022-01-06 16:39:45.000000000","message":"Overall, LGTM. Just a few doubts inline. Thanks, Heitor.","commit_id":"796a748140861f6268572384237e1477a7c8f2e5"},{"author":{"_account_id":33987,"name":"Pedro Monteiro Azevedo de Moura Almeida","display_name":"Pedro Almeida","email":"pedro.monteiroazevedodemouraalmeida@windriver.com","username":"pedroalmd"},"change_message_id":"dbb10edc1638b45d20a4223db0cdc08c6c6543a8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"d13345a0_337e68e9","updated":"2022-01-06 20:56:52.000000000","message":"LGTM","commit_id":"765c09bdf9b71cc838a4a2a12e4e565d0bd37b47"},{"author":{"_account_id":33618,"name":"Arthur Luz de Ávila","display_name":"Arthur Luz de Avila","email":"Arthur.LuzdeAvila@windriver.com","username":"luzdeav"},"change_message_id":"06ba3a200398403ae5b0c956b1e5ba8bd4bdf50a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"d9259c0e_f5cf244e","updated":"2022-01-06 21:03:34.000000000","message":"LGTM","commit_id":"765c09bdf9b71cc838a4a2a12e4e565d0bd37b47"},{"author":{"_account_id":33410,"name":"Iago Filipe","email":"IagoFilipe.EstrelaBarros@windriver.com","username":"ifest1"},"change_message_id":"146736d1cc136c455236e009b5bdb722c99f78e3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"c832142d_4f0c33ac","updated":"2022-01-07 12:40:03.000000000","message":"LGTM!","commit_id":"765c09bdf9b71cc838a4a2a12e4e565d0bd37b47"},{"author":{"_account_id":33390,"name":"Heitor Matsui","display_name":"Heitor Matsui","email":"HeitorVieira.Matsui@windriver.com","username":"heitormatsui"},"change_message_id":"cf7d7d75889991b4aa7aade7bc16211cfc88ec34","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"200350ba_4fb8f6b9","updated":"2022-01-13 14:22:44.000000000","message":"recheck","commit_id":"64761b264165291290ded543d9833c0a296c17e9"},{"author":{"_account_id":28410,"name":"Scott Little","email":"scott.little@windriver.com","username":"slittle1"},"change_message_id":"2e07ad8c43c651b6a8500435f78e3e88fb40edb8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":19,"id":"68f096f0_26c0e4b7","updated":"2022-01-14 21:40:06.000000000","message":"recheck","commit_id":"a6bfded28b5cfed60e16cd5dc2e98825cafb73b6"}],"utilities/pci-irq-affinity-agent/pci_irq_affinity/pci_irq_affinity/agent.py":[{"author":{"_account_id":14675,"name":"Hugo Nicodemos","email":"hugo.brito@windriver.com","username":"nicodemos"},"change_message_id":"1a7e75f76a0ddb54b0c835f776c1f2f5d3ff22ad","unresolved":true,"context_lines":[{"line_number":122,"context_line":"    rabbit_options \u003d CONF.amqp"},{"line_number":123,"context_line":"    try:"},{"line_number":124,"context_line":"        for option in rabbit_options:"},{"line_number":125,"context_line":"            rabbit_cfg[option] \u003d rabbit_options[option]"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"    except Exception as e:"},{"line_number":128,"context_line":"        LOG.error(\"Could not read all required rabbitmq configuration! Err\u003d%s\" % e)"}],"source_content_type":"text/x-python","patch_set":15,"id":"6978c665_eb1ead7d","line":125,"updated":"2022-01-06 16:39:45.000000000","message":"After this change, if we only clone CONF.amqp here, is it still possible for the exception happens? or could we use CONF.amqp directly","commit_id":"796a748140861f6268572384237e1477a7c8f2e5"},{"author":{"_account_id":33390,"name":"Heitor Matsui","display_name":"Heitor Matsui","email":"HeitorVieira.Matsui@windriver.com","username":"heitormatsui"},"change_message_id":"81157079b7b75247c2ca6a9ef0e53492940a2195","unresolved":false,"context_lines":[{"line_number":122,"context_line":"    rabbit_options \u003d CONF.amqp"},{"line_number":123,"context_line":"    try:"},{"line_number":124,"context_line":"        for option in rabbit_options:"},{"line_number":125,"context_line":"            rabbit_cfg[option] \u003d rabbit_options[option]"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"    except Exception as e:"},{"line_number":128,"context_line":"        LOG.error(\"Could not read all required rabbitmq configuration! Err\u003d%s\" % e)"}],"source_content_type":"text/x-python","patch_set":15,"id":"e93261a0_ff92943b","line":125,"in_reply_to":"6978c665_eb1ead7d","updated":"2022-01-06 19:22:07.000000000","message":"Done, you\u0027re right, we could remove the whole function and use CONF.amqp directly, good catch, thank you","commit_id":"796a748140861f6268572384237e1477a7c8f2e5"}],"utilities/pci-irq-affinity-agent/pci_irq_affinity/pci_irq_affinity/config.py":[{"author":{"_account_id":15435,"name":"Al Bailey","email":"albailey1974@gmail.com","username":"albailey"},"change_message_id":"39c3d815f475942f9ce5f8dd03d8d1f6d3314e4a","unresolved":true,"context_lines":[{"line_number":60,"context_line":"               help\u003d\"Set the log level for the agent\"),"},{"line_number":61,"context_line":"]"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"CONF.register_opts(openstack_opts, cfg.OptGroup(\"openstack\"))"},{"line_number":64,"context_line":"CONF.register_opts(amqp_opts, cfg.OptGroup(\"amqp\"))"},{"line_number":65,"context_line":"CONF.register_opts(parameters_opts, cfg.OptGroup(\"parameters\"))"},{"line_number":66,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"e7a8e26c_44ba0e9a","line":63,"updated":"2022-01-12 14:31:41.000000000","message":"This code is fine (We do this same thing all over the place)\nBe aware that if you try to import this file more than once (ie: two files in your project both import this config.py file)  it may blow up at runtime, since oslo is picky about register_opts being re-executed for the same entries more than once.","commit_id":"9482a298be8ea6c5fb99c7c01f09465819775bcd"},{"author":{"_account_id":33390,"name":"Heitor Matsui","display_name":"Heitor Matsui","email":"HeitorVieira.Matsui@windriver.com","username":"heitormatsui"},"change_message_id":"369e0baf221407aaa236b666a886a0f4d49b852c","unresolved":false,"context_lines":[{"line_number":60,"context_line":"               help\u003d\"Set the log level for the agent\"),"},{"line_number":61,"context_line":"]"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"CONF.register_opts(openstack_opts, cfg.OptGroup(\"openstack\"))"},{"line_number":64,"context_line":"CONF.register_opts(amqp_opts, cfg.OptGroup(\"amqp\"))"},{"line_number":65,"context_line":"CONF.register_opts(parameters_opts, cfg.OptGroup(\"parameters\"))"},{"line_number":66,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"0edb739d_f238eb87","line":63,"in_reply_to":"e7a8e26c_44ba0e9a","updated":"2022-01-14 17:39:35.000000000","message":"Thanks for the tip Al, I\u0027ll keep that in mind when using oslo_config","commit_id":"9482a298be8ea6c5fb99c7c01f09465819775bcd"}],"utilities/pci-irq-affinity-agent/pci_irq_affinity/pci_irq_affinity/driver.py":[{"author":{"_account_id":33410,"name":"Iago Filipe","email":"IagoFilipe.EstrelaBarros@windriver.com","username":"ifest1"},"change_message_id":"9514812da8651f0f79a820f900ee2aec1e065fb7","unresolved":true,"context_lines":[{"line_number":52,"context_line":"                    LOG.error(\u0027_wait_for_msi_irqs: pci_addr\u003d%(A)s, error\u003d%(E)s\u0027 %"},{"line_number":53,"context_line":"                              {\u0027A\u0027: addr, \u0027E\u0027: e})"},{"line_number":54,"context_line":"                self._msi_irq_count[addr] \u003d len(msi_irqs)"},{"line_number":55,"context_line":"                self._msi_irq_elapsed[addr] +\u003d \\"},{"line_number":56,"context_line":"                    msi_irq_check_interval"},{"line_number":57,"context_line":"                if _prev[addr] \u003d\u003d self._msi_irq_count[addr]:"},{"line_number":58,"context_line":"                    self._msi_irq_since[addr] +\u003d \\"}],"source_content_type":"text/x-python","patch_set":13,"id":"48fa771e_a7b55659","line":55,"updated":"2022-01-05 00:16:28.000000000","message":"I think we don\u0027t need this backslash.","commit_id":"293388bca562631652ca1db6a85beff5e5423192"},{"author":{"_account_id":33390,"name":"Heitor Matsui","display_name":"Heitor Matsui","email":"HeitorVieira.Matsui@windriver.com","username":"heitormatsui"},"change_message_id":"76a53ccadaf58ed17626f97a526f07df7c013853","unresolved":false,"context_lines":[{"line_number":52,"context_line":"                    LOG.error(\u0027_wait_for_msi_irqs: pci_addr\u003d%(A)s, error\u003d%(E)s\u0027 %"},{"line_number":53,"context_line":"                              {\u0027A\u0027: addr, \u0027E\u0027: e})"},{"line_number":54,"context_line":"                self._msi_irq_count[addr] \u003d len(msi_irqs)"},{"line_number":55,"context_line":"                self._msi_irq_elapsed[addr] +\u003d \\"},{"line_number":56,"context_line":"                    msi_irq_check_interval"},{"line_number":57,"context_line":"                if _prev[addr] \u003d\u003d self._msi_irq_count[addr]:"},{"line_number":58,"context_line":"                    self._msi_irq_since[addr] +\u003d \\"}],"source_content_type":"text/x-python","patch_set":13,"id":"cf4c2390_be8cd256","line":55,"in_reply_to":"48fa771e_a7b55659","updated":"2022-01-06 15:52:18.000000000","message":"Done","commit_id":"293388bca562631652ca1db6a85beff5e5423192"},{"author":{"_account_id":33410,"name":"Iago Filipe","email":"IagoFilipe.EstrelaBarros@windriver.com","username":"ifest1"},"change_message_id":"9514812da8651f0f79a820f900ee2aec1e065fb7","unresolved":true,"context_lines":[{"line_number":55,"context_line":"                self._msi_irq_elapsed[addr] +\u003d \\"},{"line_number":56,"context_line":"                    msi_irq_check_interval"},{"line_number":57,"context_line":"                if _prev[addr] \u003d\u003d self._msi_irq_count[addr]:"},{"line_number":58,"context_line":"                    self._msi_irq_since[addr] +\u003d \\"},{"line_number":59,"context_line":"                        msi_irq_check_interval"},{"line_number":60,"context_line":"                else:"},{"line_number":61,"context_line":"                    self._msi_irq_since[addr] \u003d 0"}],"source_content_type":"text/x-python","patch_set":13,"id":"d51e434c_06b8ae87","line":58,"updated":"2022-01-05 00:16:28.000000000","message":"ditto","commit_id":"293388bca562631652ca1db6a85beff5e5423192"},{"author":{"_account_id":33390,"name":"Heitor Matsui","display_name":"Heitor Matsui","email":"HeitorVieira.Matsui@windriver.com","username":"heitormatsui"},"change_message_id":"76a53ccadaf58ed17626f97a526f07df7c013853","unresolved":false,"context_lines":[{"line_number":55,"context_line":"                self._msi_irq_elapsed[addr] +\u003d \\"},{"line_number":56,"context_line":"                    msi_irq_check_interval"},{"line_number":57,"context_line":"                if _prev[addr] \u003d\u003d self._msi_irq_count[addr]:"},{"line_number":58,"context_line":"                    self._msi_irq_since[addr] +\u003d \\"},{"line_number":59,"context_line":"                        msi_irq_check_interval"},{"line_number":60,"context_line":"                else:"},{"line_number":61,"context_line":"                    self._msi_irq_since[addr] \u003d 0"}],"source_content_type":"text/x-python","patch_set":13,"id":"3ff8a23e_fd2a7fca","line":58,"in_reply_to":"d51e434c_06b8ae87","updated":"2022-01-06 15:52:18.000000000","message":"Done","commit_id":"293388bca562631652ca1db6a85beff5e5423192"}],"utilities/pci-irq-affinity-agent/pci_irq_affinity/pci_irq_affinity/nova_provider.py":[{"author":{"_account_id":14675,"name":"Hugo Nicodemos","email":"hugo.brito@windriver.com","username":"nicodemos"},"change_message_id":"1a7e75f76a0ddb54b0c835f776c1f2f5d3ff22ad","unresolved":true,"context_lines":[{"line_number":46,"context_line":"        creds_options \u003d [\u0027username\u0027, \u0027password\u0027, \u0027user_domain_name\u0027, \u0027project_name\u0027,"},{"line_number":47,"context_line":"                         \u0027project_domain_name\u0027, \u0027keyring_service\u0027, \u0027auth_url\u0027]"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"        try:"},{"line_number":50,"context_line":"            for option in creds_options:"},{"line_number":51,"context_line":"                value \u003d openstack_options[option]"},{"line_number":52,"context_line":"                if value:"}],"source_content_type":"text/x-python","patch_set":15,"id":"a67f1ccf_135fa765","line":49,"updated":"2022-01-06 16:39:45.000000000","message":"I could be wrong, but this try/except/for is just verifying if all the options in present on openstack_options, right? Can we simplify to something like:\n\n`if all(option in openstack_options for options in creds_options)`","commit_id":"796a748140861f6268572384237e1477a7c8f2e5"},{"author":{"_account_id":33390,"name":"Heitor Matsui","display_name":"Heitor Matsui","email":"HeitorVieira.Matsui@windriver.com","username":"heitormatsui"},"change_message_id":"81157079b7b75247c2ca6a9ef0e53492940a2195","unresolved":false,"context_lines":[{"line_number":46,"context_line":"        creds_options \u003d [\u0027username\u0027, \u0027password\u0027, \u0027user_domain_name\u0027, \u0027project_name\u0027,"},{"line_number":47,"context_line":"                         \u0027project_domain_name\u0027, \u0027keyring_service\u0027, \u0027auth_url\u0027]"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"        try:"},{"line_number":50,"context_line":"            for option in creds_options:"},{"line_number":51,"context_line":"                value \u003d openstack_options[option]"},{"line_number":52,"context_line":"                if value:"}],"source_content_type":"text/x-python","patch_set":15,"id":"5f317253_c773f655","line":49,"in_reply_to":"a67f1ccf_135fa765","updated":"2022-01-06 19:22:07.000000000","message":"The options in creds_options will always exist on openstack_options with this patch (as now they are all being registered on config.py), so a Exception may be thrown now only when using keyring (still true for bare metal agent) on line #56","commit_id":"796a748140861f6268572384237e1477a7c8f2e5"},{"author":{"_account_id":14675,"name":"Hugo Nicodemos","email":"hugo.brito@windriver.com","username":"nicodemos"},"change_message_id":"1a7e75f76a0ddb54b0c835f776c1f2f5d3ff22ad","unresolved":true,"context_lines":[{"line_number":52,"context_line":"                if value:"},{"line_number":53,"context_line":"                    creds[option] \u003d value"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"            if \u0027password\u0027 not in creds:"},{"line_number":56,"context_line":"                creds[\u0027password\u0027] \u003d keyring.get_password(creds[\u0027keyring_service\u0027],"},{"line_number":57,"context_line":"                                                         creds[\u0027username\u0027])"},{"line_number":58,"context_line":"            creds.pop(\u0027keyring_service\u0027)"}],"source_content_type":"text/x-python","patch_set":15,"id":"b8f85879_681a3831","line":55,"updated":"2022-01-06 16:39:45.000000000","message":"If this is true, `value \u003d openstack_options[option]` will break. Or the value could be None/empty?","commit_id":"796a748140861f6268572384237e1477a7c8f2e5"},{"author":{"_account_id":33390,"name":"Heitor Matsui","display_name":"Heitor Matsui","email":"HeitorVieira.Matsui@windriver.com","username":"heitormatsui"},"change_message_id":"81157079b7b75247c2ca6a9ef0e53492940a2195","unresolved":false,"context_lines":[{"line_number":52,"context_line":"                if value:"},{"line_number":53,"context_line":"                    creds[option] \u003d value"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"            if \u0027password\u0027 not in creds:"},{"line_number":56,"context_line":"                creds[\u0027password\u0027] \u003d keyring.get_password(creds[\u0027keyring_service\u0027],"},{"line_number":57,"context_line":"                                                         creds[\u0027username\u0027])"},{"line_number":58,"context_line":"            creds.pop(\u0027keyring_service\u0027)"}],"source_content_type":"text/x-python","patch_set":15,"id":"b122457c_8f1aa615","line":55,"in_reply_to":"b8f85879_681a3831","updated":"2022-01-06 19:22:07.000000000","message":"As I commented before, password will always exist, but possibly with None default value (for non-containerized agent, case in which it will try to get the password from the keyring), so it won\u0027t be created in creds on line #53, and it won\u0027t break","commit_id":"796a748140861f6268572384237e1477a7c8f2e5"}]}