)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3f800c89670a60d6a51f14f2776f78e41b408005","unresolved":false,"context_lines":[{"line_number":20,"context_line":""},{"line_number":21,"context_line":"* Use the value in the configuration file"},{"line_number":22,"context_line":"  if neither transport_url or database_connection is specified."},{"line_number":23,"context_line":"* Keep the value as it is if \u0027keep\u0027 is specified"},{"line_number":24,"context_line":"  for transport_url or database_connection."},{"line_number":25,"context_line":"* The command exits with a return code of 6"},{"line_number":26,"context_line":"  if either transport_url or database_connection is specified"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"9fb8cfa7_22f34876","line":23,"updated":"2019-06-12 14:57:50.000000000","message":"I\u0027m not a fan of this \"keep\" sentinel. What I would expect the behavior to be is that if I specify just one of the options but not the other, only update that one and ignore the other (don\u0027t update it from config). So then you\u0027d have these semantics:\n\n1. if you don\u0027t specify either option, update from config\n2. if you specify only one option, update from the command line option value but don\u0027t update the other from config\n3. if you specify both options from the command line, update both from the command line values\n\nThat seems to be more inline with how people expected this to work in the bug report(s) and how OSC set/unset commands work (we don\u0027t change things that you don\u0027t specify). Granted, OSC commands don\u0027t have this same kind of \"by default update from config\" behavior either.","commit_id":"6a7d1d105e68737bdebee26af82e7995218b7e4e"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"58eda6856d20ed1125dd56fea67c9e0f85d17c11","unresolved":false,"context_lines":[{"line_number":20,"context_line":""},{"line_number":21,"context_line":"* Use the value in the configuration file"},{"line_number":22,"context_line":"  if neither transport_url or database_connection is specified."},{"line_number":23,"context_line":"* Keep the value as it is if \u0027keep\u0027 is specified"},{"line_number":24,"context_line":"  for transport_url or database_connection."},{"line_number":25,"context_line":"* The command exits with a return code of 6"},{"line_number":26,"context_line":"  if either transport_url or database_connection is specified"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"9fb8cfa7_855d5e16","line":23,"in_reply_to":"9fb8cfa7_22f34876","updated":"2019-06-12 15:06:11.000000000","message":"\u003e 1. if you don\u0027t specify either option, update from config\n \u003e 2. if you specify only one option, update from the command line\n \u003e option value but don\u0027t update the other from config\n \u003e 3. if you specify both options from the command line, update both\n \u003e from the command line values\n\nThis is confusing behavior to me, because the absence of one means we don\u0027t update it, but the absence of both means we _update_ both. Completely different behavior. Further, if I encode this into my deployment tool and then it changes (again) in the future, I\u0027ve encoded an ambiguous position into my tool, which will do the wrong thing once the underlying behavior changes. Such ambiguous behavior is why we\u0027re discussing this at all.\n\nSo, IMHO, we should make it more stable, not less, regardless of what that means. I can think of other changes we could make to get towards more stability (i.e. gating the config-using behavior on a --from-config which takes everything from the config, exclusive of the other two) but that will make existing users have to change more as well.","commit_id":"6a7d1d105e68737bdebee26af82e7995218b7e4e"}],"doc/source/cli/nova-manage.rst":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"46fd37565dcd7e7d061fe01c9d1d37888dfdde8c","unresolved":false,"context_lines":[{"line_number":295,"context_line":""},{"line_number":296,"context_line":"``nova-manage cell_v2 update_cell --cell_uuid \u003ccell_uuid\u003e [--name \u003ccell_name\u003e] [--transport-url \u003ctransport_url\u003e] [--database_connection \u003cdatabase_connection\u003e] [--disable] [--enable]``"},{"line_number":297,"context_line":"    Updates the properties of a cell by the given uuid."},{"line_number":298,"context_line":"    If neither database_connection or transport_url is not specified,"},{"line_number":299,"context_line":"    it will attempt to use the values defined by ``[database]/connection`` and"},{"line_number":300,"context_line":"    ``[DEFAULT]/transport_url`` in the configuration file."},{"line_number":301,"context_line":"    If \u0027keep\u0027 is specified for database_connection or transport_url,"}],"source_content_type":"text/x-rst","patch_set":3,"id":"9fb8cfa7_fc7bac24","line":298,"range":{"start_line":298,"start_character":55,"end_line":298,"end_character":58},"updated":"2019-06-11 14:15:29.000000000","message":"s/not//","commit_id":"0866439ad097e7914068767133352b6bd2d5ddc8"},{"author":{"_account_id":7634,"name":"Takashi Natsume","email":"takanattie@gmail.com","username":"natsumet"},"change_message_id":"61ce5a5cd58ae9fb95ce0c30de8756dbc18c39e3","unresolved":false,"context_lines":[{"line_number":295,"context_line":""},{"line_number":296,"context_line":"``nova-manage cell_v2 update_cell --cell_uuid \u003ccell_uuid\u003e [--name \u003ccell_name\u003e] [--transport-url \u003ctransport_url\u003e] [--database_connection \u003cdatabase_connection\u003e] [--disable] [--enable]``"},{"line_number":297,"context_line":"    Updates the properties of a cell by the given uuid."},{"line_number":298,"context_line":"    If neither database_connection or transport_url is not specified,"},{"line_number":299,"context_line":"    it will attempt to use the values defined by ``[database]/connection`` and"},{"line_number":300,"context_line":"    ``[DEFAULT]/transport_url`` in the configuration file."},{"line_number":301,"context_line":"    If \u0027keep\u0027 is specified for database_connection or transport_url,"}],"source_content_type":"text/x-rst","patch_set":3,"id":"9fb8cfa7_26960f40","line":298,"range":{"start_line":298,"start_character":55,"end_line":298,"end_character":58},"in_reply_to":"9fb8cfa7_fc7bac24","updated":"2019-06-11 19:03:53.000000000","message":"Done","commit_id":"0866439ad097e7914068767133352b6bd2d5ddc8"}],"nova/cmd/manage.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"9440b194ab10d3e418ef732d47c03f05459d1d7e","unresolved":false,"context_lines":[{"line_number":1514,"context_line":"            cell_mapping.name \u003d name"},{"line_number":1515,"context_line":""},{"line_number":1516,"context_line":"        transport_url \u003d transport_url or CONF.transport_url"},{"line_number":1517,"context_line":"        db_connection \u003d db_connection or CONF.database.connection"},{"line_number":1518,"context_line":""},{"line_number":1519,"context_line":"        if (self._non_unique_transport_url_database_connection_checker(ctxt,"},{"line_number":1520,"context_line":"                cell_mapping, transport_url, db_connection)):"}],"source_content_type":"text/x-python","patch_set":1,"id":"bfb3d3c7_c34d5ac4","side":"PARENT","line":1517,"updated":"2019-05-31 17:39:29.000000000","message":"Instead of removing this completely, I think you need to update them to default to the found cell_mapping, as you described in your commit message, example:\n\n transport_url \u003d transport_url or cell_mapping.transport_url\n db_connection \u003d db_connection or cell_mapping.database_connection\n\nOtherwise, we will not be checking for an already existing CellMapping with different uuid but same transport_url and db_connection properly on L1519.","commit_id":"6e4ab9714cc0ca147f61997aa7b68f88185ade5c"},{"author":{"_account_id":7634,"name":"Takashi Natsume","email":"takanattie@gmail.com","username":"natsumet"},"change_message_id":"d0169a03e5d3800a70d9d977111335a35c84fa62","unresolved":false,"context_lines":[{"line_number":1514,"context_line":"            cell_mapping.name \u003d name"},{"line_number":1515,"context_line":""},{"line_number":1516,"context_line":"        transport_url \u003d transport_url or CONF.transport_url"},{"line_number":1517,"context_line":"        db_connection \u003d db_connection or CONF.database.connection"},{"line_number":1518,"context_line":""},{"line_number":1519,"context_line":"        if (self._non_unique_transport_url_database_connection_checker(ctxt,"},{"line_number":1520,"context_line":"                cell_mapping, transport_url, db_connection)):"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_9c4f1a33","side":"PARENT","line":1517,"in_reply_to":"9fb8cfa7_75218afb","updated":"2019-06-11 03:58:28.000000000","message":"Done","commit_id":"6e4ab9714cc0ca147f61997aa7b68f88185ade5c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e7b80adf085305189e8d4a4bb4eb53d23e74689b","unresolved":false,"context_lines":[{"line_number":1514,"context_line":"            cell_mapping.name \u003d name"},{"line_number":1515,"context_line":""},{"line_number":1516,"context_line":"        transport_url \u003d transport_url or CONF.transport_url"},{"line_number":1517,"context_line":"        db_connection \u003d db_connection or CONF.database.connection"},{"line_number":1518,"context_line":""},{"line_number":1519,"context_line":"        if (self._non_unique_transport_url_database_connection_checker(ctxt,"},{"line_number":1520,"context_line":"                cell_mapping, transport_url, db_connection)):"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_75218afb","side":"PARENT","line":1517,"in_reply_to":"9fb8cfa7_f0e29601","updated":"2019-06-04 14:48:23.000000000","message":"Yeah, the point of this is so that you can run this against a config file and not specify credentials on the command line.\n\nOne thing we could do is something like this:\n\n if transport_url or db_connection:\n     # At least one argument was provided, require the other\n     # to be specified, or a sentinel to leave it unchanged\n     if transport_url \u003d\u003d \u0027keep\u0027:\n         transport_url \u003d cell_mapping.transport_url\n     elif transport_url is None:\n         print(\u0027Specify a transport_url or \"keep\" to leave unchanged\u0027)\n     if db_connection \u003d\u003d \u0027keep\u0027:\n         db_connection \u003d cell_mapping.db_connection\n     elif db_connection is None:\n         print(\u0027Specify a db_connection or \"keep\" to leave unchanged\u0027)\n else:\n     # No command-line arguments, so take the config values\n     database_connection \u003d CONF.database.connection\n     transport_url \u003d CONF.transport_url\n\nThat would make the behavior either \"you want to set this from command-line arguments, so tell me both values or which one(s) to leave unchanged\" or \"You want me to take both values from the config\".\n\nI think that\u0027s easy to grok and explain in a help message. You could also add an \"if \u003d\u003d conf\" option for the argument case, but I think it\u0027s unnecessary and probably just increases confusion.","commit_id":"6e4ab9714cc0ca147f61997aa7b68f88185ade5c"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"b5f2716d3a4c1f18abf1c45a60fc6a8538d6a84c","unresolved":false,"context_lines":[{"line_number":1514,"context_line":"            cell_mapping.name \u003d name"},{"line_number":1515,"context_line":""},{"line_number":1516,"context_line":"        transport_url \u003d transport_url or CONF.transport_url"},{"line_number":1517,"context_line":"        db_connection \u003d db_connection or CONF.database.connection"},{"line_number":1518,"context_line":""},{"line_number":1519,"context_line":"        if (self._non_unique_transport_url_database_connection_checker(ctxt,"},{"line_number":1520,"context_line":"                cell_mapping, transport_url, db_connection)):"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_f0e29601","side":"PARENT","line":1517,"in_reply_to":"bfb3d3c7_c34d5ac4","updated":"2019-05-31 18:38:53.000000000","message":"Hm, I just found this old review where a similar change was proposed and it was rejected because apparently it\u0027s intentional that these default to the config file values:\n\nhttps://review.opendev.org/#/c/603998/2/nova/cmd/manage.py\n\nIf that is still the case, it looks like the best we could do here is add code comments explaining that the default to the config values is intentional and why. Because it\u0027s not intuitive, at least to me.","commit_id":"6e4ab9714cc0ca147f61997aa7b68f88185ade5c"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"4f7f4b69432b4cdc6ea19da0b26eab47ddd38893","unresolved":false,"context_lines":[{"line_number":1519,"context_line":"            # semantic meanings of return codes."},{"line_number":1520,"context_line":"            return 3"},{"line_number":1521,"context_line":""},{"line_number":1522,"context_line":"        if transport_url:"},{"line_number":1523,"context_line":"            cell_mapping.transport_url \u003d transport_url"},{"line_number":1524,"context_line":""},{"line_number":1525,"context_line":"        if db_connection:"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_f065b6e2","line":1522,"updated":"2019-05-31 18:19:25.000000000","message":"And here we should check if transport_url !\u003d cell_mapping.transport_url to avoid a needless update of the field in the case that it\u0027s not different (since we will default to the existing value).","commit_id":"45eed1339dc7a1b8c69d1bb41cd46ac057c5ccd9"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"4f7f4b69432b4cdc6ea19da0b26eab47ddd38893","unresolved":false,"context_lines":[{"line_number":1522,"context_line":"        if transport_url:"},{"line_number":1523,"context_line":"            cell_mapping.transport_url \u003d transport_url"},{"line_number":1524,"context_line":""},{"line_number":1525,"context_line":"        if db_connection:"},{"line_number":1526,"context_line":"            cell_mapping.database_connection \u003d db_connection"},{"line_number":1527,"context_line":""},{"line_number":1528,"context_line":"        if disable and enable:"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_90623ad8","line":1525,"updated":"2019-05-31 18:19:25.000000000","message":"Same.","commit_id":"45eed1339dc7a1b8c69d1bb41cd46ac057c5ccd9"}],"nova/tests/unit/test_nova_manage.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"9440b194ab10d3e418ef732d47c03f05459d1d7e","unresolved":false,"context_lines":[{"line_number":2023,"context_line":"            uuidsentinel.cell1, \u0027foo\u0027, \u0027fake://new\u0027, \u0027fake:///new\u0027))"},{"line_number":2024,"context_line":"        self.assertIn(\u0027not found\u0027, self.output.getvalue())"},{"line_number":2025,"context_line":""},{"line_number":2026,"context_line":"    def test_update_cell_failed_if_non_unique_transport_db_urls(self):"},{"line_number":2027,"context_line":"        ctxt \u003d context.get_admin_context()"},{"line_number":2028,"context_line":"        objects.CellMapping(context\u003dctxt, uuid\u003duuidsentinel.cell1,"},{"line_number":2029,"context_line":"                            name\u003d\u0027cell1\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_8d6225d8","line":2026,"updated":"2019-05-31 17:39:29.000000000","message":"I think this test needs additional coverage since it didn\u0027t catch the problem and fail on PS1 with not defaulting to the specified CellMapping\u0027s transport_url and database_connection.","commit_id":"45eed1339dc7a1b8c69d1bb41cd46ac057c5ccd9"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"9440b194ab10d3e418ef732d47c03f05459d1d7e","unresolved":false,"context_lines":[{"line_number":2050,"context_line":""},{"line_number":2051,"context_line":"        cell2_update4 \u003d self.commands.update_cell("},{"line_number":2052,"context_line":"            uuidsentinel.cell2, \u0027foo\u0027, \u0027fake://mq3\u0027, \u0027fake:///db3\u0027)"},{"line_number":2053,"context_line":"        self.assertEqual(0, cell2_update4)"},{"line_number":2054,"context_line":""},{"line_number":2055,"context_line":"    def test_update_cell_failed(self):"},{"line_number":2056,"context_line":"        ctxt \u003d context.get_admin_context()"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_4d83cd57","line":2053,"updated":"2019-05-31 17:39:29.000000000","message":"I think we need two more cases here, where only one of transport_url or database_connection is specified instead of both. As far as I can tell, the update_cell command does not require a user to pass both options. With PS1, I believe that these cases would erroneously allow an update that would result in a duplicate DB and MQ for two cells with different UUIDs.\n\nSo I think we need a case where update_cell(..., None, \u003cdb url\u003e) is called and another case where update_cell(..., \u003cmq url\u003e, None) is called.","commit_id":"45eed1339dc7a1b8c69d1bb41cd46ac057c5ccd9"}]}