)]}'
{"nova/virt/ironic/driver.py":[{"author":{"_account_id":14358,"name":"Marian Horban","email":"m.horban@gmail.com","username":"mhorban"},"change_message_id":"55d64e13ddb5ab4b1cc3963611cb7ab6933ffa8d","unresolved":false,"context_lines":[{"line_number":126,"context_line":"        return power_state.NOSTATE"},{"line_number":127,"context_line":""},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"def api_version_higher_than(version):"},{"line_number":130,"context_line":"    \"\"\"Check if version is higher than the specified version.\"\"\""},{"line_number":131,"context_line":"    config_version \u003d str(float(CONF.ironic.api_version))"},{"line_number":132,"context_line":"    return StrictVersion(config_version) \u003e StrictVersion(version)"}],"source_content_type":"text/x-python","patch_set":1,"id":"da20952f_4ffc9fb4","line":129,"updated":"2015-09-07 22:39:36.000000000","message":"Why not api_version_higher_than_OR_EQUAL(). If feature is implemented in 1.8 version it looks more clear to call\n api_version_higher_than_or_eq(\u00271.8\u0027)","commit_id":"c03d2b30c414056c06b59f70899d4e8331806880"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"62850d5ea8e12b202a7ffe45609918cd48b08cdd","unresolved":false,"context_lines":[{"line_number":126,"context_line":"        return power_state.NOSTATE"},{"line_number":127,"context_line":""},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"def api_version_higher_than(version):"},{"line_number":130,"context_line":"    \"\"\"Check if version is higher than the specified version.\"\"\""},{"line_number":131,"context_line":"    config_version \u003d str(float(CONF.ironic.api_version))"},{"line_number":132,"context_line":"    return StrictVersion(config_version) \u003e StrictVersion(version)"}],"source_content_type":"text/x-python","patch_set":1,"id":"da20952f_55338413","line":129,"in_reply_to":"da20952f_4ffc9fb4","updated":"2015-09-09 15:53:35.000000000","message":"++ will do","commit_id":"c03d2b30c414056c06b59f70899d4e8331806880"},{"author":{"_account_id":14358,"name":"Marian Horban","email":"m.horban@gmail.com","username":"mhorban"},"change_message_id":"55d64e13ddb5ab4b1cc3963611cb7ab6933ffa8d","unresolved":false,"context_lines":[{"line_number":520,"context_line":"            node_list \u003d []"},{"line_number":521,"context_line":"        return node_list"},{"line_number":522,"context_line":""},{"line_number":523,"context_line":"    def _get_node(self, node_ident):"},{"line_number":524,"context_line":"        extra_params \u003d {}"},{"line_number":525,"context_line":"        # NOTE(lucasagomes): For API versions \u003e 1.7 Ironic supports fetching"},{"line_number":526,"context_line":"        # only specific fields of a given resource"}],"source_content_type":"text/x-python","patch_set":1,"id":"da20952f_0fd8e751","line":523,"updated":"2015-09-07 22:39:36.000000000","message":"Looks like _get_node and _validate_instance_and_node similar. _validate_instance_and_node should use _get_node inside try ... except wrapper.","commit_id":"c03d2b30c414056c06b59f70899d4e8331806880"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"62850d5ea8e12b202a7ffe45609918cd48b08cdd","unresolved":false,"context_lines":[{"line_number":520,"context_line":"            node_list \u003d []"},{"line_number":521,"context_line":"        return node_list"},{"line_number":522,"context_line":""},{"line_number":523,"context_line":"    def _get_node(self, node_ident):"},{"line_number":524,"context_line":"        extra_params \u003d {}"},{"line_number":525,"context_line":"        # NOTE(lucasagomes): For API versions \u003e 1.7 Ironic supports fetching"},{"line_number":526,"context_line":"        # only specific fields of a given resource"}],"source_content_type":"text/x-python","patch_set":1,"id":"da20952f_f080a6f9","line":523,"in_reply_to":"da20952f_0fd8e751","updated":"2015-09-09 15:53:35.000000000","message":"Done","commit_id":"c03d2b30c414056c06b59f70899d4e8331806880"},{"author":{"_account_id":10343,"name":"Jim Rollenhagen","email":"jim@jimrollenhagen.com","username":"jimrollenhagen"},"change_message_id":"97e816423038ef89036f3e9415267e3cb809e72d","unresolved":false,"context_lines":[{"line_number":118,"context_line":""},{"line_number":119,"context_line":"_NODE_FIELDS \u003d (\u0027uuid\u0027, \u0027power_state\u0027, \u0027target_power_state\u0027, \u0027provision_state\u0027,"},{"line_number":120,"context_line":"                \u0027target_provision_state\u0027, \u0027last_error\u0027, \u0027maintenance\u0027,"},{"line_number":121,"context_line":"                \u0027properties\u0027, \u0027instance_uuid\u0027)"},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"_IRONIC_API_VERSION \u003d None"},{"line_number":124,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"da20952f_cb9d88bc","line":121,"updated":"2015-09-10 14:11:57.000000000","message":"So I generally like this approach, but there\u0027s a \"race condition\" here so to speak.\n\nImagine we break out capabilities into its own table or field in the API. If we update Ironic first, Nova won\u0027t get the capabilities in this API call. If we update Nova first, I assume the Ironic API validates the fields passed and API calls will fail.\n\nThis is solvable by storing things in both places while they move, but something to think about.","commit_id":"0a406d13c42b9dd691b9cb95b5a6164a2a2bf1bf"},{"author":{"_account_id":10343,"name":"Jim Rollenhagen","email":"jim@jimrollenhagen.com","username":"jimrollenhagen"},"change_message_id":"6c929ad76e14d3184af35eb1483d7886b6004896","unresolved":false,"context_lines":[{"line_number":118,"context_line":""},{"line_number":119,"context_line":"_NODE_FIELDS \u003d (\u0027uuid\u0027, \u0027power_state\u0027, \u0027target_power_state\u0027, \u0027provision_state\u0027,"},{"line_number":120,"context_line":"                \u0027target_provision_state\u0027, \u0027last_error\u0027, \u0027maintenance\u0027,"},{"line_number":121,"context_line":"                \u0027properties\u0027, \u0027instance_uuid\u0027)"},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"_IRONIC_API_VERSION \u003d None"},{"line_number":124,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"ba15a1d1_ad59b590","line":121,"in_reply_to":"da20952f_b1ebe611","updated":"2015-09-12 20:52:37.000000000","message":"Yeah, +1, seems fine.","commit_id":"0a406d13c42b9dd691b9cb95b5a6164a2a2bf1bf"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"0238f81c91171fb5b2fca72fb3c60902549d9013","unresolved":false,"context_lines":[{"line_number":118,"context_line":""},{"line_number":119,"context_line":"_NODE_FIELDS \u003d (\u0027uuid\u0027, \u0027power_state\u0027, \u0027target_power_state\u0027, \u0027provision_state\u0027,"},{"line_number":120,"context_line":"                \u0027target_provision_state\u0027, \u0027last_error\u0027, \u0027maintenance\u0027,"},{"line_number":121,"context_line":"                \u0027properties\u0027, \u0027instance_uuid\u0027)"},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"_IRONIC_API_VERSION \u003d None"},{"line_number":124,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"da20952f_ce76f505","line":121,"in_reply_to":"da20952f_cb9d88bc","updated":"2015-09-10 15:00:15.000000000","message":"That\u0027s what microversion is for right?\n\nIf we break capabilities own it\u0027s own table or field in the API we are going to introduce a new version of the API in Ironic. But older versions should continue to work as before. So there won\u0027t be any problem updating Ironic first because the Nova Ironic driver will be asking for an older version of the API where the capabilities still being part of the properties fields.\n\nSame if we update the code in Nova first, the configuration option will be pinned on a older version of the Ironic API, so things should just work independently of which service you update first.\n\n...\n\nBut say we do this change in Ironic API, for nova we need to add some logic on the _get_node() function.\n\n if api_version.minor \u003e\u003d X:\n   _NODE_FIELDS +\u003d [\u0027capabilities\u0027]\n\n(or something like that)","commit_id":"0a406d13c42b9dd691b9cb95b5a6164a2a2bf1bf"},{"author":{"_account_id":10343,"name":"Jim Rollenhagen","email":"jim@jimrollenhagen.com","username":"jimrollenhagen"},"change_message_id":"06d0cef301f0ee482dac4d1f53360ad41926be85","unresolved":false,"context_lines":[{"line_number":118,"context_line":""},{"line_number":119,"context_line":"_NODE_FIELDS \u003d (\u0027uuid\u0027, \u0027power_state\u0027, \u0027target_power_state\u0027, \u0027provision_state\u0027,"},{"line_number":120,"context_line":"                \u0027target_provision_state\u0027, \u0027last_error\u0027, \u0027maintenance\u0027,"},{"line_number":121,"context_line":"                \u0027properties\u0027, \u0027instance_uuid\u0027)"},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"_IRONIC_API_VERSION \u003d None"},{"line_number":124,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"da20952f_ee7bb9bc","line":121,"in_reply_to":"da20952f_ce76f505","updated":"2015-09-10 15:03:17.000000000","message":"Oh, good point :)\n\nThis now makes me wonder... should we support \u0027latest\u0027 in the api_version config option?","commit_id":"0a406d13c42b9dd691b9cb95b5a6164a2a2bf1bf"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"6528da59c8a1d4e1788067f1d5ae902b24170bbb","unresolved":false,"context_lines":[{"line_number":118,"context_line":""},{"line_number":119,"context_line":"_NODE_FIELDS \u003d (\u0027uuid\u0027, \u0027power_state\u0027, \u0027target_power_state\u0027, \u0027provision_state\u0027,"},{"line_number":120,"context_line":"                \u0027target_provision_state\u0027, \u0027last_error\u0027, \u0027maintenance\u0027,"},{"line_number":121,"context_line":"                \u0027properties\u0027, \u0027instance_uuid\u0027)"},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"_IRONIC_API_VERSION \u003d None"},{"line_number":124,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"da20952f_b1ebe611","line":121,"in_reply_to":"da20952f_ee7bb9bc","updated":"2015-09-10 15:07:46.000000000","message":"Yeah, I thought about it before, tho it\u0027s complicated. Because it\u0027s easy to someone shot themselves on the foot with the \"latest\", especially with upgrades as your comment points out.\n\nAlso, I don\u0027t see the point of the driver to be passing \"latest\" because it won\u0027t have any code to deal with the latest features from Ironic? The driver needs to implement support for the latest features before it starts using it.","commit_id":"0a406d13c42b9dd691b9cb95b5a6164a2a2bf1bf"},{"author":{"_account_id":10343,"name":"Jim Rollenhagen","email":"jim@jimrollenhagen.com","username":"jimrollenhagen"},"change_message_id":"97e816423038ef89036f3e9415267e3cb809e72d","unresolved":false,"context_lines":[{"line_number":128,"context_line":"    if _IRONIC_API_VERSION is None:"},{"line_number":129,"context_line":"        _IRONIC_API_VERSION \u003d version.IronicAPIVersion(CONF.ironic.api_version)"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"    return _IRONIC_API_VERSION"},{"line_number":132,"context_line":""},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"def map_power_state(state):"}],"source_content_type":"text/x-python","patch_set":2,"id":"da20952f_300cb541","line":131,"updated":"2015-09-10 14:11:57.000000000","message":"Why not just use self._api_version rather than a global and whatnot?","commit_id":"0a406d13c42b9dd691b9cb95b5a6164a2a2bf1bf"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"0238f81c91171fb5b2fca72fb3c60902549d9013","unresolved":false,"context_lines":[{"line_number":128,"context_line":"    if _IRONIC_API_VERSION is None:"},{"line_number":129,"context_line":"        _IRONIC_API_VERSION \u003d version.IronicAPIVersion(CONF.ironic.api_version)"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"    return _IRONIC_API_VERSION"},{"line_number":132,"context_line":""},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"def map_power_state(state):"}],"source_content_type":"text/x-python","patch_set":2,"id":"da20952f_4e0e65e6","line":131,"in_reply_to":"da20952f_300cb541","updated":"2015-09-10 15:00:15.000000000","message":"Because the _validate_instance_and_node() function is not part of the Ironic driver class and it uses this version object. Also, I think it\u0027s OK to be a constant because the configuration option is loaded only once, so we only need to instantiate the version object once as well.","commit_id":"0a406d13c42b9dd691b9cb95b5a6164a2a2bf1bf"},{"author":{"_account_id":10343,"name":"Jim Rollenhagen","email":"jim@jimrollenhagen.com","username":"jimrollenhagen"},"change_message_id":"06d0cef301f0ee482dac4d1f53360ad41926be85","unresolved":false,"context_lines":[{"line_number":128,"context_line":"    if _IRONIC_API_VERSION is None:"},{"line_number":129,"context_line":"        _IRONIC_API_VERSION \u003d version.IronicAPIVersion(CONF.ironic.api_version)"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"    return _IRONIC_API_VERSION"},{"line_number":132,"context_line":""},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"def map_power_state(state):"}],"source_content_type":"text/x-python","patch_set":2,"id":"da20952f_ce9275f6","line":131,"in_reply_to":"da20952f_4e0e65e6","updated":"2015-09-10 15:03:17.000000000","message":"We should promote that method to a class method, then, or drop self._api_version. Doesn\u0027t make sense to me to have two sources of truth for that.","commit_id":"0a406d13c42b9dd691b9cb95b5a6164a2a2bf1bf"},{"author":{"_account_id":10343,"name":"Jim Rollenhagen","email":"jim@jimrollenhagen.com","username":"jimrollenhagen"},"change_message_id":"97e816423038ef89036f3e9415267e3cb809e72d","unresolved":false,"context_lines":[{"line_number":139,"context_line":"        return power_state.NOSTATE"},{"line_number":140,"context_line":""},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"def _get_node(ironicclient, ident, instance\u003dFalse):"},{"line_number":143,"context_line":"    extra_params \u003d {}"},{"line_number":144,"context_line":"    # NOTE(lucasagomes): For API versions \u003e\u003d 1.8 Ironic supports fetching"},{"line_number":145,"context_line":"    # only specific fields of a given resource"}],"source_content_type":"text/x-python","patch_set":2,"id":"da20952f_cb010806","line":142,"updated":"2015-09-10 14:11:57.000000000","message":"instance\u003dNone?","commit_id":"0a406d13c42b9dd691b9cb95b5a6164a2a2bf1bf"},{"author":{"_account_id":10343,"name":"Jim Rollenhagen","email":"jim@jimrollenhagen.com","username":"jimrollenhagen"},"change_message_id":"06d0cef301f0ee482dac4d1f53360ad41926be85","unresolved":false,"context_lines":[{"line_number":139,"context_line":"        return power_state.NOSTATE"},{"line_number":140,"context_line":""},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"def _get_node(ironicclient, ident, instance\u003dFalse):"},{"line_number":143,"context_line":"    extra_params \u003d {}"},{"line_number":144,"context_line":"    # NOTE(lucasagomes): For API versions \u003e\u003d 1.8 Ironic supports fetching"},{"line_number":145,"context_line":"    # only specific fields of a given resource"}],"source_content_type":"text/x-python","patch_set":2,"id":"da20952f_6eea4976","line":142,"in_reply_to":"da20952f_0e3aadcd","updated":"2015-09-10 15:03:17.000000000","message":"Aha! I see... maybe just \"is_instance\"","commit_id":"0a406d13c42b9dd691b9cb95b5a6164a2a2bf1bf"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"0238f81c91171fb5b2fca72fb3c60902549d9013","unresolved":false,"context_lines":[{"line_number":139,"context_line":"        return power_state.NOSTATE"},{"line_number":140,"context_line":""},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"def _get_node(ironicclient, ident, instance\u003dFalse):"},{"line_number":143,"context_line":"    extra_params \u003d {}"},{"line_number":144,"context_line":"    # NOTE(lucasagomes): For API versions \u003e\u003d 1.8 Ironic supports fetching"},{"line_number":145,"context_line":"    # only specific fields of a given resource"}],"source_content_type":"text/x-python","patch_set":2,"id":"da20952f_0e3aadcd","line":142,"in_reply_to":"da20952f_cb010806","updated":"2015-09-10 15:00:15.000000000","message":"Yeah, the parameter name \"instance\" is not great, it\u0027s just to indicate whether the \"id\" passed is an instance id or node id.\n\nPerhaps that parameter should be called \"is_instance_id\u003dFalse/True\"","commit_id":"0a406d13c42b9dd691b9cb95b5a6164a2a2bf1bf"},{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"b322da08b9887308592659785862da87b37ce820","unresolved":false,"context_lines":[{"line_number":148,"context_line":"    # NOTE(lucasagomes): For API versions \u003e\u003d 1.8 Ironic supports fetching"},{"line_number":149,"context_line":"    # only specific fields of a given resource"},{"line_number":150,"context_line":"    api_version \u003d get_ironic_api_version()"},{"line_number":151,"context_line":"    if api_version.major \u003d\u003d 1 and api_version.minor \u003e\u003d 8:"},{"line_number":152,"context_line":"        extra_params[\"fields\"] \u003d _NODE_FIELDS"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"    cmd \u003d \"node.get_by_instance_uuid\" if instance else \"node.get\""}],"source_content_type":"text/x-python","patch_set":4,"id":"3a7e1126_56edcfed","line":151,"updated":"2015-12-17 19:48:48.000000000","message":"yeh, I don\u0027t want to do this at all. The point of the microversion mechanism is that the client (in this case Nova) can write to 1 version and not have all this conditional code in here, because the contract is known.\n\nSo make this a const for 1.8, put in the release notes that you need Ironic 1.8 to use this version of nova, and let\u0027s be done with it.","commit_id":"ec88d7296e0c3c0b00c3058b3b1db9d5c10625be"}]}