)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fd9348be56bad6b71af808c805dda77cec1cf064","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Change If625411f40be0ba642baeb02950f568f43673655 introduced"},{"line_number":10,"context_line":"nova.utils.get_ksa_adapter, which accepts min_version and max_version"},{"line_number":11,"context_line":"kwargs to be passed through to the ksa Adapter constructor. These are"},{"line_number":12,"context_line":"supposed to represent minimum and maximum *major* API versions,"},{"line_number":13,"context_line":"min_version was erroneously set to *microversions* when setting up the"},{"line_number":14,"context_line":"Adapter for ironicclient. This commit changes it to a major version."},{"line_number":15,"context_line":"(Microversion negotiation is done within ironicclient itself.)"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"ffb9cba7_db0e62e4","line":12,"range":{"start_line":12,"start_character":42,"end_line":12,"end_character":49},"updated":"2019-04-23 19:23:24.000000000","message":"Yup, which is all over the ksa docs:\n\nhttps://docs.openstack.org/keystoneauth/latest/using-sessions.html#service-discovery\n\nhttps://docs.openstack.org/keystoneauth/latest/api/keystoneauth1.html#keystoneauth1.adapter.Adapter\n\nThe compute API examples probably aren\u0027t super helpful in making this clear since the min_version used there is 2.1 (the cinder examples are more clear where min_version\u003d3).","commit_id":"13278be9f265e237fc68ee60acfacaa1df68522e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fd9348be56bad6b71af808c805dda77cec1cf064","unresolved":false,"context_lines":[{"line_number":14,"context_line":"Adapter for ironicclient. This commit changes it to a major version."},{"line_number":15,"context_line":"(Microversion negotiation is done within ironicclient itself.)"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Also, this bug went latent for several releases because a) it only seems"},{"line_number":18,"context_line":"to be triggered when region_name is given in the conf; but also b)"},{"line_number":19,"context_line":"ironicclient has code to discover a reasonable endpoint if passed None."},{"line_number":20,"context_line":"So this change also adds a warning log if we try and fail to discover"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"ffb9cba7_bbf5aecd","line":17,"updated":"2019-04-23 19:23:24.000000000","message":"You could also mention that (c) nova is not doing microversion negotiation with any other service except cinder and we\u0027re not using ksa adapters for that:\n\nhttps://review.opendev.org/#/c/508345/","commit_id":"13278be9f265e237fc68ee60acfacaa1df68522e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fd9348be56bad6b71af808c805dda77cec1cf064","unresolved":false,"context_lines":[{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Also, this bug went latent for several releases because a) it only seems"},{"line_number":18,"context_line":"to be triggered when region_name is given in the conf; but also b)"},{"line_number":19,"context_line":"ironicclient has code to discover a reasonable endpoint if passed None."},{"line_number":20,"context_line":"So this change also adds a warning log if we try and fail to discover"},{"line_number":21,"context_line":"the endpoint via ksa."},{"line_number":22,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"ffb9cba7_5b1b521a","line":19,"range":{"start_line":19,"start_character":56,"end_line":19,"end_character":70},"updated":"2019-04-23 19:23:24.000000000","message":"passed None for what? The region_name?","commit_id":"13278be9f265e237fc68ee60acfacaa1df68522e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e04cbebd4c23c76cf243a86f86476e81418c723b","unresolved":false,"context_lines":[{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Also, this bug went latent for several releases because a) it only seems"},{"line_number":18,"context_line":"to be triggered when region_name is given in the conf; but also b)"},{"line_number":19,"context_line":"ironicclient has code to discover a reasonable endpoint if passed None."},{"line_number":20,"context_line":"So this change also adds a warning log if we try and fail to discover"},{"line_number":21,"context_line":"the endpoint via ksa."},{"line_number":22,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"ffb9cba7_1bbcfada","line":19,"range":{"start_line":19,"start_character":56,"end_line":19,"end_character":70},"in_reply_to":"ffb9cba7_5b1b521a","updated":"2019-04-23 19:31:42.000000000","message":"api_endpoint, sorry","commit_id":"13278be9f265e237fc68ee60acfacaa1df68522e"}],"nova/virt/ironic/client_wrapper.py":[{"author":{"_account_id":22623,"name":"Erik Olof Gunnar Andersson","email":"eandersson@blizzard.com","username":"eoandersson"},"change_message_id":"124d90c13bd462e60c405dbbcd75e2d93f4c7448","unresolved":false,"context_lines":[{"line_number":129,"context_line":"                ironic_url_none_reason \u003d \u0027raised ServiceNotFound\u0027"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"            if ironic_url is None:"},{"line_number":132,"context_line":"                LOG.warning(\"Could not discover ironic_url via keystoneauth1: \""},{"line_number":133,"context_line":"                            \"Adapter.get_endpoint \" + ironic_url_none_reason)"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":1,"id":"ffb9cba7_dfd74e23","line":132,"updated":"2019-04-22 16:19:55.000000000","message":"This should be lazy loaded.\n\u003e \"Adapter.get_endpoint %s\", ironic_url_none_reason)","commit_id":"2341f82533effbd21c993199b8f01953f1e0e7c4"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"2aa37880764f811641c821129e75e9ad10a75c7c","unresolved":false,"context_lines":[{"line_number":129,"context_line":"                ironic_url_none_reason \u003d \u0027raised ServiceNotFound\u0027"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"            if ironic_url is None:"},{"line_number":132,"context_line":"                LOG.warning(\"Could not discover ironic_url via keystoneauth1: \""},{"line_number":133,"context_line":"                            \"Adapter.get_endpoint \" + ironic_url_none_reason)"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":1,"id":"ffb9cba7_a2a91de0","line":132,"in_reply_to":"ffb9cba7_dfd74e23","updated":"2019-04-22 16:41:23.000000000","message":"whoops, yup","commit_id":"2341f82533effbd21c993199b8f01953f1e0e7c4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fd9348be56bad6b71af808c805dda77cec1cf064","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        # accepted was added in python-ironicclient 2.2.0. The highest"},{"line_number":97,"context_line":"        # available version will be utilized by the client for the lifetime"},{"line_number":98,"context_line":"        # of the client."},{"line_number":99,"context_line":"        kwargs[\u0027os_ironic_api_version\u0027] \u003d ["},{"line_number":100,"context_line":"            \u0027%d.%d\u0027 % IRONIC_API_VERSION, \u0027%d.%d\u0027 % PRIOR_IRONIC_API_VERSION]"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"        ironic_conf \u003d CONF[IRONIC_GROUP.name]"}],"source_content_type":"text/x-python","patch_set":2,"id":"ffb9cba7_7bff36aa","line":99,"updated":"2019-04-23 19:23:24.000000000","message":"OK and this is a microversion.","commit_id":"13278be9f265e237fc68ee60acfacaa1df68522e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fd9348be56bad6b71af808c805dda77cec1cf064","unresolved":false,"context_lines":[{"line_number":114,"context_line":"            ironic_url \u003d CONF.ironic.api_endpoint"},{"line_number":115,"context_line":"        else:"},{"line_number":116,"context_line":"            try:"},{"line_number":117,"context_line":"                ksa_adap \u003d utils.get_ksa_adapter("},{"line_number":118,"context_line":"                    nova.conf.ironic.DEFAULT_SERVICE_TYPE,"},{"line_number":119,"context_line":"                    ksa_auth\u003dauth_plugin, ksa_session\u003dsess,"},{"line_number":120,"context_line":"                    min_version\u003d(IRONIC_API_VERSION[0], 0),"}],"source_content_type":"text/x-python","patch_set":2,"id":"ffb9cba7_bb1e0e0a","line":117,"updated":"2019-04-23 19:23:24.000000000","message":"https://docs.openstack.org/keystoneauth/latest/using-sessions.html#endpoint-metadata says there is also a min_microversion kwarg, why don\u0027t we use that? Or is ironicclient already dealing with that because of os_ironic_api_version above?","commit_id":"13278be9f265e237fc68ee60acfacaa1df68522e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e04cbebd4c23c76cf243a86f86476e81418c723b","unresolved":false,"context_lines":[{"line_number":114,"context_line":"            ironic_url \u003d CONF.ironic.api_endpoint"},{"line_number":115,"context_line":"        else:"},{"line_number":116,"context_line":"            try:"},{"line_number":117,"context_line":"                ksa_adap \u003d utils.get_ksa_adapter("},{"line_number":118,"context_line":"                    nova.conf.ironic.DEFAULT_SERVICE_TYPE,"},{"line_number":119,"context_line":"                    ksa_auth\u003dauth_plugin, ksa_session\u003dsess,"},{"line_number":120,"context_line":"                    min_version\u003d(IRONIC_API_VERSION[0], 0),"}],"source_content_type":"text/x-python","patch_set":2,"id":"ffb9cba7_bbccce89","line":117,"in_reply_to":"ffb9cba7_bb1e0e0a","updated":"2019-04-23 19:31:42.000000000","message":"There\u0027s not a min_microversion kwarg. The doc says that ksa can do some discovery to figure out the min and max microversion supported by the API. AFAIK there isn\u0027t actually a way to do microversion bounding at ksa loading (which is what get_ksa_adapter does) or get_endpoint. And yes, actual microversion negotiation is done by ironicclient via os_ironic_api_version.","commit_id":"13278be9f265e237fc68ee60acfacaa1df68522e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fd9348be56bad6b71af808c805dda77cec1cf064","unresolved":false,"context_lines":[{"line_number":117,"context_line":"                ksa_adap \u003d utils.get_ksa_adapter("},{"line_number":118,"context_line":"                    nova.conf.ironic.DEFAULT_SERVICE_TYPE,"},{"line_number":119,"context_line":"                    ksa_auth\u003dauth_plugin, ksa_session\u003dsess,"},{"line_number":120,"context_line":"                    min_version\u003d(IRONIC_API_VERSION[0], 0),"},{"line_number":121,"context_line":"                    max_version\u003d(IRONIC_API_VERSION[0], ks_disc.LATEST))"},{"line_number":122,"context_line":"                ironic_url \u003d ksa_adap.get_endpoint()"},{"line_number":123,"context_line":"                ironic_url_none_reason \u003d \u0027returned None\u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"ffb9cba7_fbe1e6ff","line":120,"range":{"start_line":120,"start_character":20,"end_line":120,"end_character":31},"updated":"2019-04-23 19:23:24.000000000","message":"Looks like we don\u0027t have to change any docs here either:\n\nhttps://github.com/openstack/nova/blob/fef4696e005b12c2316e652d17d2310d573dd0e7/nova/utils.py#L1182","commit_id":"13278be9f265e237fc68ee60acfacaa1df68522e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fd9348be56bad6b71af808c805dda77cec1cf064","unresolved":false,"context_lines":[{"line_number":128,"context_line":"                ironic_url \u003d None"},{"line_number":129,"context_line":"                ironic_url_none_reason \u003d \u0027raised ServiceNotFound\u0027"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"            if ironic_url is None:"},{"line_number":132,"context_line":"                LOG.warning(\"Could not discover ironic_url via keystoneauth1: \""},{"line_number":133,"context_line":"                            \"Adapter.get_endpoint %s\", ironic_url_none_reason)"},{"line_number":134,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"ffb9cba7_5bf032ca","line":131,"updated":"2019-04-23 19:23:24.000000000","message":"So what happens if we pass endpoint\u003dironic_url to ironicclient? Will it blow up? Or do it\u0027s own version discovery? I\u0027m mostly worried about a change in behavior here and then backporting this change.","commit_id":"13278be9f265e237fc68ee60acfacaa1df68522e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e04cbebd4c23c76cf243a86f86476e81418c723b","unresolved":false,"context_lines":[{"line_number":128,"context_line":"                ironic_url \u003d None"},{"line_number":129,"context_line":"                ironic_url_none_reason \u003d \u0027raised ServiceNotFound\u0027"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"            if ironic_url is None:"},{"line_number":132,"context_line":"                LOG.warning(\"Could not discover ironic_url via keystoneauth1: \""},{"line_number":133,"context_line":"                            \"Adapter.get_endpoint %s\", ironic_url_none_reason)"},{"line_number":134,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"ffb9cba7_fbafa691","line":131,"in_reply_to":"ffb9cba7_5bf032ca","updated":"2019-04-23 19:31:42.000000000","message":"Yeah, ahem, the change in behavior is that we\u0027ll now be actually doing what we said we were doing in queens. The ironic_url we (now) pass in should wind up being the same as whatever ironicclient was guessing at before when we passed None. And per the comment on L125, the code was structured to \"fail in a backward compatible\" way if the ksa lookup yielded None.","commit_id":"13278be9f265e237fc68ee60acfacaa1df68522e"}]}
