)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"d545ba5e8a57f4a9f801ae611579e46dd87f0b11","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"b10e6a1e_8f730f33","updated":"2021-12-03 07:10:11.000000000","message":"Thanks for the fix. Would there be an upgrade impact with this? Can you please add a release note? https://docs.openstack.org/manila/latest/contributor/adding_release_notes.html","commit_id":"4195f0b7f847244c677f4489b88eac30d1076725"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"184d8b76f232214326a05ee7c195eb6a29bf486a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"c6a383c0_b755feab","in_reply_to":"2324cfef_bd5b6061","updated":"2021-12-15 20:22:16.000000000","message":"\u003e At the 2nd sight (kudos to Takashi!), not sure about upgrade impact. Perhaps updating this parameter should happen consistently, otherwise two sides of RPC wouldn\u0027t be able to communicate, until restarted with a new config? Also, it might affect custom ha-policies set by operators for rabbitmq, expecting \u0027openstack\u0027 and not \u0027manila\u0027. Given that said, I look forward for the experts\u0027 feedback on that and how to move this forward.\n\n\u003e The status quo is - one cannot use amqp durable queues with such control_exchange defaults, and the feature remains (and will remain) effectively blocked for years.\n\nYes, this has been my understanding as well. To extend this:\n\nDo we need the exchanges to be isolated between services because other configuration options relating to amqp could conflict between the services that share an exchange? \n\nOr would it be because you want the topic publishers and subscribers to be isolated per service for best practice? i.e., durable queues allow rabbit agents to all die, and on revival, recover queue contents from memory. So if there are unrelated publishers/subscribers is there any risk of this recovery not happening?\n\n\nI ask these questions to understand the impact of bricking manila components on an upgrade. When upgrading a cloud, we already prevent communication between newer and older services by virtue of the RPC API Version. So, if the default exchange has changed, there\u0027s no real risk that an \"older\" (un-upgraded) service can no longer communicate with a newer manila service.  However, \"newer\" services could always pick up and process messages from \"older\" services - with this change, they will be on a different exchange, so they won\u0027t see any messages. This is the real upgrade impact, correct? Is there _any_ way at all that we could avoid this? What do you mean by \n\n\u003e Perhaps updating this parameter should happen consistently, otherwise two sides of RPC wouldn\u0027t be able to communicate, until restarted with a new config?","commit_id":"4195f0b7f847244c677f4489b88eac30d1076725"},{"author":{"_account_id":6926,"name":"Bogdan Dobrelya","email":"bdobreli@redhat.com","username":"bogdando"},"change_message_id":"e1381e72602fed3e246fa45acc4b9b163285857b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"a18bdb5c_d8570c51","in_reply_to":"79bbd833_22c73221","updated":"2021-12-08 10:19:39.000000000","message":"At the 2nd sight (kudos to Takashi!), not sure about upgrade impact. Perhaps updating this parameter should happen consistently, otherwise two sides of RPC wouldn\u0027t be able to communicate, until restarted with a new config? Also, it might affect custom ha-policies set by operators for rabbitmq, expecting \u0027openstack\u0027 and not \u0027manila\u0027. Given that said, I look forward for the experts\u0027 feedback on that and how to move this forward.\n\nThe status quo is - one cannot use amqp durable queues with such control_exchange defaults, and the feature remains (and will remain) effectively blocked for years.","commit_id":"4195f0b7f847244c677f4489b88eac30d1076725"},{"author":{"_account_id":31721,"name":"Felipe Rodrigues","email":"felipefuty01@gmail.com","username":"felipefutty"},"change_message_id":"855fe4b4e5217ef2e3e93756cc1c016464e77cef","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"2324cfef_bd5b6061","in_reply_to":"a18bdb5c_d8570c51","updated":"2021-12-12 00:27:09.000000000","message":"Sorry Bogdan, I did not get it! So, changing the default from \u0027openstack\u0027 to \u0027manila\u0027 won\u0027t break any user, right ?!","commit_id":"4195f0b7f847244c677f4489b88eac30d1076725"},{"author":{"_account_id":6926,"name":"Bogdan Dobrelya","email":"bdobreli@redhat.com","username":"bogdando"},"change_message_id":"42d6b5f0dc2f35153091e3d5cfe2c9acad99c446","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"79bbd833_22c73221","in_reply_to":"b10e6a1e_8f730f33","updated":"2021-12-08 09:12:46.000000000","message":"Done.\nNo upgrade impact since the change is internal to the messaging mechanics.","commit_id":"4195f0b7f847244c677f4489b88eac30d1076725"},{"author":{"_account_id":6926,"name":"Bogdan Dobrelya","email":"bdobreli@redhat.com","username":"bogdando"},"change_message_id":"84064e60e94d7de8d3788c7a201771a0ab1d07b4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"37a1b410_f182ae56","in_reply_to":"c6a383c0_b755feab","updated":"2021-12-21 10:10:45.000000000","message":"\u003e However, \"newer\" services could always pick up and process messages from \"older\" services - with this change, they will be on a different exchange, so they won\u0027t see any messages. This is the real upgrade impact, correct? Is there _any_ way at all that we could avoid this?\n\nyes, but not for the projects, but oslo.messaging. It should provide a fallback mechanism to cover cases when that \"old\" default exchange disappears. For example, some dispatcher there could introduce by default twice shared control exchanges, one durable by default and the other not, and a switch to decide to publish on one or the other (a dispatcher).","commit_id":"4195f0b7f847244c677f4489b88eac30d1076725"},{"author":{"_account_id":31721,"name":"Felipe Rodrigues","email":"felipefuty01@gmail.com","username":"felipefutty"},"change_message_id":"855fe4b4e5217ef2e3e93756cc1c016464e77cef","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"ee573c24_b61d969f","updated":"2021-12-12 00:27:09.000000000","message":"Thank you Bogdan! Just some comments about the units.","commit_id":"be61b315683cda3669d92f1a98c1c16a82cbed5d"},{"author":{"_account_id":31721,"name":"Felipe Rodrigues","email":"felipefuty01@gmail.com","username":"felipefutty"},"change_message_id":"7f9363da15ac461114fc892ae027913d7a904cdf","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"dfd0cefb_ea50e524","updated":"2021-12-12 00:27:21.000000000","message":"ops, my vote.","commit_id":"be61b315683cda3669d92f1a98c1c16a82cbed5d"}],"manila/common/config.py":[{"author":{"_account_id":31721,"name":"Felipe Rodrigues","email":"felipefuty01@gmail.com","username":"felipefutty"},"change_message_id":"855fe4b4e5217ef2e3e93756cc1c016464e77cef","unresolved":true,"context_lines":[{"line_number":182,"context_line":"    # Update default value of oslo.policy policy_file config option."},{"line_number":183,"context_line":"    policy_opts.set_defaults(CONF, \u0027policy.yaml\u0027)"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    # Update default value of RPC transport control_exchange config option."},{"line_number":186,"context_line":"    rpc.set_defaults(control_exchange\u003d\u0027manila\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"4857673f_5bbfa58e","line":186,"range":{"start_line":185,"start_character":3,"end_line":186,"end_character":47},"updated":"2021-12-12 00:27:09.000000000","message":"The unit test that you implemented does not cover the added line [1]\n\nhttps://c77aa8f87e095b99b6e6-3a828353f8ae4adcb13d9c2988d28730.ssl.cf2.rackcdn.com/820197/2/check/openstack-tox-cover/53828bf/cover/d_93790f427c40867c_config_py.html","commit_id":"be61b315683cda3669d92f1a98c1c16a82cbed5d"}],"manila/tests/test_rpc.py":[{"author":{"_account_id":31721,"name":"Felipe Rodrigues","email":"felipefuty01@gmail.com","username":"felipefutty"},"change_message_id":"855fe4b4e5217ef2e3e93756cc1c016464e77cef","unresolved":true,"context_lines":[{"line_number":38,"context_line":"        self.assertTrue(messaging_mock.Notifier.called)"},{"line_number":39,"context_line":"        self.assertEqual(rpc.NOTIFIER, messaging_mock.Notifier.return_value)"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"    @mock.patch(\u0027manila.rpc.set_defaults\u0027)"},{"line_number":42,"context_line":"    def test_set_defaults(self, mock_set):"},{"line_number":43,"context_line":"        control_exchange \u003d mock.Mock()"},{"line_number":44,"context_line":"        rpc.set_defaults(control_exchange)"},{"line_number":45,"context_line":"        mock_set.assert_called_once_with(control_exchange)"}],"source_content_type":"text/x-python","patch_set":2,"id":"8fc931e5_1fb7dfb8","line":45,"range":{"start_line":41,"start_character":4,"end_line":45,"end_character":58},"updated":"2021-12-12 00:27:09.000000000","message":"I think that this code is not covering anything. Actually, if you see the cover result [1] the body of method \"set_defaults\" from RPC is not been covered. Look, the patch is not adding the method, it is using it, right? So, you don\u0027t need to cover it here. What you need to cover is the new line common/config.py (it might not be convered entirely, though). However, if you desire to cover the \"set_defaults\" method (thanks), it should be something like:\n\n@mock.patch(rpc, \u0027messaging\u0027)\ndef test_set_defaults(self, messaging_mock):\n  rpc.set_defaults(\u0027test\u0027)\n  messaging_mock.assert_called_once_with(\u0027test\u0027)\n\n\n[1] https://c77aa8f87e095b99b6e6-3a828353f8ae4adcb13d9c2988d28730.ssl.cf2.rackcdn.com/820197/2/check/openstack-tox-cover/53828bf/cover/d_5ccf1bd44142e8a2_rpc_py.html#t85","commit_id":"be61b315683cda3669d92f1a98c1c16a82cbed5d"}]}