)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":29100,"name":"Jason Anderson","email":"jasonanderson@uchicago.edu","username":"jasonanderson"},"change_message_id":"795868d414bd1bb488a9140902a2b380c11e3c3c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"4de36b92_08977a4b","updated":"2022-01-14 16:34:50.000000000","message":"In general looks good, had some questions about possible edge cases though!","commit_id":"6d8ead1d2eccfa2b65ed63898b6405393327ddf0"},{"author":{"_account_id":33582,"name":"Mark Powers","email":"markpowers@uchicago.edu","username":"markpowers"},"change_message_id":"784213444fe42b42869824710df12cd03fa8b870","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"512f1830_3ded1136","updated":"2022-02-03 22:43:24.000000000","message":"Looks good!","commit_id":"3d01888c14a785e796393d7f6a2b0792b1b1f2e0"}],"blazarnova/scheduler/filters/blazar_filter.py":[{"author":{"_account_id":29100,"name":"Jason Anderson","email":"jasonanderson@uchicago.edu","username":"jasonanderson"},"change_message_id":"795868d414bd1bb488a9140902a2b380c11e3c3c","unresolved":true,"context_lines":[{"line_number":64,"context_line":"        aggregates \u003d host_state.aggregates"},{"line_number":65,"context_line":"        pools \u003d []"},{"line_number":66,"context_line":"        for agg in aggregates:"},{"line_number":67,"context_line":"            if (agg.availability_zone and"},{"line_number":68,"context_line":"                    str(agg.availability_zone).startswith("},{"line_number":69,"context_line":"                        cfg.CONF[\u0027blazar:physical:host\u0027].blazar_az_prefix)"},{"line_number":70,"context_line":"                    # NOTE(hiro-kobayashi): following 2 lines are for keeping"}],"source_content_type":"text/x-python","patch_set":3,"id":"b0379d4f_dae30dd0","line":67,"updated":"2022-01-14 16:34:50.000000000","message":"Was this a long-dormant bug?","commit_id":"6d8ead1d2eccfa2b65ed63898b6405393327ddf0"},{"author":{"_account_id":29100,"name":"Jason Anderson","email":"jasonanderson@uchicago.edu","username":"jasonanderson"},"change_message_id":"4e13f9034c10c398e5ecf2152cffea32b5381048","unresolved":false,"context_lines":[{"line_number":64,"context_line":"        aggregates \u003d host_state.aggregates"},{"line_number":65,"context_line":"        pools \u003d []"},{"line_number":66,"context_line":"        for agg in aggregates:"},{"line_number":67,"context_line":"            if (agg.availability_zone and"},{"line_number":68,"context_line":"                    str(agg.availability_zone).startswith("},{"line_number":69,"context_line":"                        cfg.CONF[\u0027blazar:physical:host\u0027].blazar_az_prefix)"},{"line_number":70,"context_line":"                    # NOTE(hiro-kobayashi): following 2 lines are for keeping"}],"source_content_type":"text/x-python","patch_set":3,"id":"f694ff30_3e02121a","line":67,"in_reply_to":"83445311_fc0c3569","updated":"2022-01-18 22:02:59.000000000","message":"Ack","commit_id":"6d8ead1d2eccfa2b65ed63898b6405393327ddf0"},{"author":{"_account_id":15197,"name":"Pierre Riteau","email":"pierre@stackhpc.com","username":"priteau","status":"StackHPC"},"change_message_id":"eca091ed45c865750daeb0b01edd53523c941ec1","unresolved":true,"context_lines":[{"line_number":64,"context_line":"        aggregates \u003d host_state.aggregates"},{"line_number":65,"context_line":"        pools \u003d []"},{"line_number":66,"context_line":"        for agg in aggregates:"},{"line_number":67,"context_line":"            if (agg.availability_zone and"},{"line_number":68,"context_line":"                    str(agg.availability_zone).startswith("},{"line_number":69,"context_line":"                        cfg.CONF[\u0027blazar:physical:host\u0027].blazar_az_prefix)"},{"line_number":70,"context_line":"                    # NOTE(hiro-kobayashi): following 2 lines are for keeping"}],"source_content_type":"text/x-python","patch_set":3,"id":"83445311_fc0c3569","line":67,"in_reply_to":"b0379d4f_dae30dd0","updated":"2022-01-17 14:34:59.000000000","message":"It may be only triggered by unit tests when the aggregate object is incompletely mocked. But it\u0027s safer this way.","commit_id":"6d8ead1d2eccfa2b65ed63898b6405393327ddf0"},{"author":{"_account_id":29100,"name":"Jason Anderson","email":"jasonanderson@uchicago.edu","username":"jasonanderson"},"change_message_id":"795868d414bd1bb488a9140902a2b380c11e3c3c","unresolved":true,"context_lines":[{"line_number":78,"context_line":""},{"line_number":79,"context_line":"            if agg.name \u003d\u003d ("},{"line_number":80,"context_line":"                    cfg.CONF[\u0027blazar:physical:host\u0027].preemptible_aggregate):"},{"line_number":81,"context_line":"                pools.append(agg)"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"        return pools"},{"line_number":84,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"69335513_6379f475","line":81,"updated":"2022-01-14 16:34:50.000000000","message":"If for some reason the aggregate pool and the preemptible pool are set to the same name, the aggregate will get added to the pools list twice, which I think may cause weird behavior later on when the length of the pools object is checked. Maybe it should be a set, or the existence of the aggregate should be checked before inserting again? Or, maybe this is an invalid configuration, at which point it could issue a warning?","commit_id":"6d8ead1d2eccfa2b65ed63898b6405393327ddf0"},{"author":{"_account_id":15197,"name":"Pierre Riteau","email":"pierre@stackhpc.com","username":"priteau","status":"StackHPC"},"change_message_id":"eca091ed45c865750daeb0b01edd53523c941ec1","unresolved":true,"context_lines":[{"line_number":78,"context_line":""},{"line_number":79,"context_line":"            if agg.name \u003d\u003d ("},{"line_number":80,"context_line":"                    cfg.CONF[\u0027blazar:physical:host\u0027].preemptible_aggregate):"},{"line_number":81,"context_line":"                pools.append(agg)"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"        return pools"},{"line_number":84,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"8cc49e66_1ffdad5d","line":81,"in_reply_to":"69335513_6379f475","updated":"2022-01-17 14:34:59.000000000","message":"Using the same name would allow using the freepool directly (with support in Blazar for the same approach). I will update.","commit_id":"6d8ead1d2eccfa2b65ed63898b6405393327ddf0"},{"author":{"_account_id":15197,"name":"Pierre Riteau","email":"pierre@stackhpc.com","username":"priteau","status":"StackHPC"},"change_message_id":"3593c0ce61e9281f98dcce2ba88b10c670b29cd7","unresolved":false,"context_lines":[{"line_number":78,"context_line":""},{"line_number":79,"context_line":"            if agg.name \u003d\u003d ("},{"line_number":80,"context_line":"                    cfg.CONF[\u0027blazar:physical:host\u0027].preemptible_aggregate):"},{"line_number":81,"context_line":"                pools.append(agg)"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"        return pools"},{"line_number":84,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"e6794565_cb889e40","line":81,"in_reply_to":"8cc49e66_1ffdad5d","updated":"2022-02-03 20:51:39.000000000","message":"I updated the patch and reverted to use the freepool by default. Additional features linked to the use of a dedicated preemptibles aggregate will come in separate patches.","commit_id":"6d8ead1d2eccfa2b65ed63898b6405393327ddf0"},{"author":{"_account_id":29100,"name":"Jason Anderson","email":"jasonanderson@uchicago.edu","username":"jasonanderson"},"change_message_id":"795868d414bd1bb488a9140902a2b380c11e3c3c","unresolved":true,"context_lines":[{"line_number":149,"context_line":"            return True"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"        allow_preempt \u003d cfg.CONF[\u0027blazar:physical:host\u0027].allow_preemptibles"},{"line_number":152,"context_line":"        preemptible \u003d bool_from_string("},{"line_number":153,"context_line":"            extra_specs.get(FLAVOR_PREEMPTIBLE, False))"},{"line_number":154,"context_line":"        blazar_pools \u003d self.fetch_blazar_pools(host_state)"},{"line_number":155,"context_line":"        # If the request is for a preemptible instance and they are allowed"}],"source_content_type":"text/x-python","patch_set":3,"id":"a403633a_9b3f7142","line":152,"updated":"2022-01-14 16:34:50.000000000","message":"Should FLAVOR_PREEMPTIBLE also be a configurable flavor name? It seems that the operator would have to create a new flavor if they want to use this, would it be convenient for them to designate an existing flavor instead?","commit_id":"6d8ead1d2eccfa2b65ed63898b6405393327ddf0"},{"author":{"_account_id":15197,"name":"Pierre Riteau","email":"pierre@stackhpc.com","username":"priteau","status":"StackHPC"},"change_message_id":"eca091ed45c865750daeb0b01edd53523c941ec1","unresolved":true,"context_lines":[{"line_number":149,"context_line":"            return True"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"        allow_preempt \u003d cfg.CONF[\u0027blazar:physical:host\u0027].allow_preemptibles"},{"line_number":152,"context_line":"        preemptible \u003d bool_from_string("},{"line_number":153,"context_line":"            extra_specs.get(FLAVOR_PREEMPTIBLE, False))"},{"line_number":154,"context_line":"        blazar_pools \u003d self.fetch_blazar_pools(host_state)"},{"line_number":155,"context_line":"        # If the request is for a preemptible instance and they are allowed"}],"source_content_type":"text/x-python","patch_set":3,"id":"f986b223_2861310b","line":152,"in_reply_to":"a403633a_9b3f7142","updated":"2022-01-17 14:34:59.000000000","message":"FLAVOR_PREEMPTIBLE is the name of a property that can be added onto any flavor. Nova doesn\u0027t allow customise property names used by the scheduler (for example aggregate_instance_extra_specs), I went with the same approach.","commit_id":"6d8ead1d2eccfa2b65ed63898b6405393327ddf0"},{"author":{"_account_id":29100,"name":"Jason Anderson","email":"jasonanderson@uchicago.edu","username":"jasonanderson"},"change_message_id":"4e13f9034c10c398e5ecf2152cffea32b5381048","unresolved":false,"context_lines":[{"line_number":149,"context_line":"            return True"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"        allow_preempt \u003d cfg.CONF[\u0027blazar:physical:host\u0027].allow_preemptibles"},{"line_number":152,"context_line":"        preemptible \u003d bool_from_string("},{"line_number":153,"context_line":"            extra_specs.get(FLAVOR_PREEMPTIBLE, False))"},{"line_number":154,"context_line":"        blazar_pools \u003d self.fetch_blazar_pools(host_state)"},{"line_number":155,"context_line":"        # If the request is for a preemptible instance and they are allowed"}],"source_content_type":"text/x-python","patch_set":3,"id":"f68d908e_26d8906f","line":152,"in_reply_to":"f986b223_2861310b","updated":"2022-01-18 22:02:59.000000000","message":"Ah OK, I misunderstood.","commit_id":"6d8ead1d2eccfa2b65ed63898b6405393327ddf0"},{"author":{"_account_id":29100,"name":"Jason Anderson","email":"jasonanderson@uchicago.edu","username":"jasonanderson"},"change_message_id":"37408b961e9099432fed302bd0d54105fc129d29","unresolved":true,"context_lines":[{"line_number":151,"context_line":"        blazar_pools \u003d self.fetch_blazar_pools(host_state)"},{"line_number":152,"context_line":"        # If the request is for a preemptible instance and they are allowed"},{"line_number":153,"context_line":"        if allow_preempt and preemptible:"},{"line_number":154,"context_line":"            if (len(blazar_pools) \u003d\u003d 1 and blazar_pools[0].name \u003d\u003d"},{"line_number":155,"context_line":"                    cfg.CONF[\u0027blazar:physical:host\u0027].preemptible_aggregate):"},{"line_number":156,"context_line":"                # Pass host if it only belongs to the preemptibles aggregate"},{"line_number":157,"context_line":"                LOG.info(\"Host %s allowed for preemptibles\" % host_state)"}],"source_content_type":"text/x-python","patch_set":4,"id":"cdbd70c4_e37a6c21","line":154,"updated":"2022-02-04 15:44:11.000000000","message":"A check for explicit length of 1 is good b/c it\u0027s safer, but under what circumstances would this be of length \u003e 1? Feels like that\u0027s the result of a bug. Not a comment for here perhaps, but I think there\u0027s a case for a periodic task in Blazar that performs cleanup on aggregate state to get around this. Otherwise there will be a host in the freepool but it won\u0027t be getting scheduled for preemptibles (b/c it\u0027s also in another aggregate), and this will be far from obvious to the operator.","commit_id":"3d01888c14a785e796393d7f6a2b0792b1b1f2e0"},{"author":{"_account_id":15197,"name":"Pierre Riteau","email":"pierre@stackhpc.com","username":"priteau","status":"StackHPC"},"change_message_id":"301b50575e7028537a31a0a354507fb87bd50740","unresolved":false,"context_lines":[{"line_number":151,"context_line":"        blazar_pools \u003d self.fetch_blazar_pools(host_state)"},{"line_number":152,"context_line":"        # If the request is for a preemptible instance and they are allowed"},{"line_number":153,"context_line":"        if allow_preempt and preemptible:"},{"line_number":154,"context_line":"            if (len(blazar_pools) \u003d\u003d 1 and blazar_pools[0].name \u003d\u003d"},{"line_number":155,"context_line":"                    cfg.CONF[\u0027blazar:physical:host\u0027].preemptible_aggregate):"},{"line_number":156,"context_line":"                # Pass host if it only belongs to the preemptibles aggregate"},{"line_number":157,"context_line":"                LOG.info(\"Host %s allowed for preemptibles\" % host_state)"}],"source_content_type":"text/x-python","patch_set":4,"id":"886505ac_82834d66","line":154,"in_reply_to":"cdbd70c4_e37a6c21","updated":"2022-02-07 16:42:18.000000000","message":"I think this was future-proofing for instance reservation support: in instance reservation mode, hosts are both in the freepool and in reservation aggregates.\n\nHowever I just tested and realised that fetch_blazar_pools() filters aggregates with blazar_az_prefix, causing only physical reservation aggregates to be included.","commit_id":"3d01888c14a785e796393d7f6a2b0792b1b1f2e0"}]}