)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"adc296ddc4e1e11fd19099e2fd56a303a0864696","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     Arkady Shtempler \u003carkadysh@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2023-10-29 17:54:43 +0200"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Two minor changes in test_dns_integration.py code"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"1) Add an option to configure \"segmentation_id\""},{"line_number":10,"context_line":"https://docs.openstack.org/neutron/latest/admin/config-dns-int-ext"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"b20cb8f8_bc25cc9a","line":7,"updated":"2023-10-31 14:50:56.000000000","message":"The fact you have to describe the patch as \"two changes\" suggests these should be two separate patches.\n\nAlso, the commit message title should explain the intent. It\u0027s unclear what the intent is from reading the title.\n\nThis is covered in https://wiki.openstack.org/wiki/GitCommitMessages#Information_in_commit_messages\n\n(Finally, the title is incorrect - the changes touch not just test_dns_integration.py.)","commit_id":"132799fa5e9906d384b72adda7b75e04c36e0006"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"adc296ddc4e1e11fd19099e2fd56a303a0864696","unresolved":true,"context_lines":[{"line_number":7,"context_line":"Two minor changes in test_dns_integration.py code"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"1) Add an option to configure \"segmentation_id\""},{"line_number":10,"context_line":"https://docs.openstack.org/neutron/latest/admin/config-dns-int-ext"},{"line_number":11,"context_line":"-serv.html#configuration-of-the-externally-accessible-network-for"},{"line_number":12,"context_line":"-use-cases-3b-and-3c"},{"line_number":13,"context_line":"2) Skip the tests if a \"whole\" designate tempest plugin is missing"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"aface469_90951345","line":10,"updated":"2023-10-31 14:50:56.000000000","message":"I don\u0027t see how this link is relevant to the patch now that we established that 12345 is a good a ID as 100001.\n\nWhat the patch does is not enabling validation of a particular use case, but it allows the person who runs tempest to configure a VXLAN ID to be used in designate test suite (The goal for the overridden ID would be to hopefully not overlap with the VNI range configured in neutron config file - or via segments API - for \"regular\" (project) networks - in case the range is larger than [1:12344].) Note that the default range configured in gate is [1:2000] and as-is is not affected.\n\n^ This should be explained in the commit message.","commit_id":"132799fa5e9906d384b72adda7b75e04c36e0006"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"adc296ddc4e1e11fd19099e2fd56a303a0864696","unresolved":true,"context_lines":[{"line_number":10,"context_line":"https://docs.openstack.org/neutron/latest/admin/config-dns-int-ext"},{"line_number":11,"context_line":"-serv.html#configuration-of-the-externally-accessible-network-for"},{"line_number":12,"context_line":"-use-cases-3b-and-3c"},{"line_number":13,"context_line":"2) Skip the tests if a \"whole\" designate tempest plugin is missing"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Change-Id: I1d07151bae1b68c74443ed78302ee5b835f8c4dc"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"e5ee5790_da596714","line":13,"updated":"2023-10-31 14:50:56.000000000","message":"This is actually incorrect approach, see my other comment in code. (This is why it\u0027s better not to merge changes doing separate things into one - it slows down merging of pieces that are ready.)","commit_id":"132799fa5e9906d384b72adda7b75e04c36e0006"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":28609,"name":"Arkady Shtempler","email":"ashtempl@redhat.com","username":"ashtempl"},"change_message_id":"8b140cc7676ac88e163c0294a752ef1dc4de0489","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"4b206e99_fa3e927c","updated":"2023-09-13 09:43:46.000000000","message":"recheck","commit_id":"759725fe15787c2b569f6d3ce0236272cd829446"},{"author":{"_account_id":28609,"name":"Arkady Shtempler","email":"ashtempl@redhat.com","username":"ashtempl"},"change_message_id":"acfb018268178310885dd9e1582c6b1919a44b05","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"6d97e8ae_eda67494","updated":"2023-09-14 05:03:48.000000000","message":"recheck","commit_id":"759725fe15787c2b569f6d3ce0236272cd829446"},{"author":{"_account_id":28609,"name":"Arkady Shtempler","email":"ashtempl@redhat.com","username":"ashtempl"},"change_message_id":"557d77b58d36d0475eadb0467245d5666bc9ac05","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"25db0ed8_4d793d1d","updated":"2023-09-11 11:22:55.000000000","message":"recheck\nRecheck the results","commit_id":"759725fe15787c2b569f6d3ce0236272cd829446"},{"author":{"_account_id":8313,"name":"Lajos Katona","display_name":"lajoskatona","email":"katonalala@gmail.com","username":"elajkat","status":"Ericsson Software Technology"},"change_message_id":"8a4cb219d049e9b87e6e7196e898012aea7f5d8a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"69bd2712_17b5b316","in_reply_to":"6d97e8ae_eda67494","updated":"2023-09-14 08:57:28.000000000","message":"Please do not do bare recheck, always check the logs, and give a reason for rehceck","commit_id":"759725fe15787c2b569f6d3ce0236272cd829446"},{"author":{"_account_id":28609,"name":"Arkady Shtempler","email":"ashtempl@redhat.com","username":"ashtempl"},"change_message_id":"58f5fb9afeccc2b93ebdcc0abb6f8116a005b46c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"23757ed3_ef0bac55","updated":"2023-10-03 09:22:14.000000000","message":"Failed \"openvswitch gates\" are seem to be related to this bug:\nhttps://bugs.launchpad.net/neutron/+bug/2037239\nSo this patch is another example fro known problem","commit_id":"dc37be501d88fb7a63470536ed760b7f6f8f370e"},{"author":{"_account_id":28609,"name":"Arkady Shtempler","email":"ashtempl@redhat.com","username":"ashtempl"},"change_message_id":"fb6627181d96e39f107a54ebf55042199f44679b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"7310fca1_7a78005c","updated":"2023-10-02 19:27:04.000000000","message":"recheck\n\nFailed tests doesn\u0027t seem to be related to the patch\u0027s code changes:\n{0} neutron_tempest_plugin.scenario.test_basic.NetworkBasicTest.test_basic_instance [603.550857s] ... FAILED\n{1} setUpClass (neutron_tempest_plugin.scenario.test_floatingip.FloatingIPQosTest) [0.000000s] ... FAILED\n{2} tempest.api.compute.servers.test_attach_interfaces.AttachInterfacesUnderV243Test.test_add_remove_fixed_ip [329.062525s] ... FAILED","commit_id":"dc37be501d88fb7a63470536ed760b7f6f8f370e"},{"author":{"_account_id":8313,"name":"Lajos Katona","display_name":"lajoskatona","email":"katonalala@gmail.com","username":"elajkat","status":"Ericsson Software Technology"},"change_message_id":"79e49b45aad4509c50a27f7f21dad88d83239ede","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"90244bb1_51520476","updated":"2023-10-10 13:24:38.000000000","message":"Looks ok, thanks","commit_id":"a7a4fcc5060253cfdf897fea644faee50df10d48"},{"author":{"_account_id":28609,"name":"Arkady Shtempler","email":"ashtempl@redhat.com","username":"ashtempl"},"change_message_id":"b03dd1c21b6f5a5ece705585602d3564b9ca8d81","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"f97e2d14_de6c0f6e","updated":"2023-10-28 08:11:42.000000000","message":"Thank you Ihar for the review 👍","commit_id":"a7a4fcc5060253cfdf897fea644faee50df10d48"},{"author":{"_account_id":28609,"name":"Arkady Shtempler","email":"ashtempl@redhat.com","username":"ashtempl"},"change_message_id":"ed0e5e5a2bcfd77ebf79fe8cf1baf24a09374b85","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"f22227fb_cb38c345","updated":"2023-10-10 06:50:20.000000000","message":"recheck\n\nRecheck the result, a single failed gate: \"neutron-tempest-plugin-scenario-ovn-yoga \" was OK in a previous attempts and the last failure is not related to the code changes made in this patch.","commit_id":"a7a4fcc5060253cfdf897fea644faee50df10d48"},{"author":{"_account_id":28609,"name":"Arkady Shtempler","email":"ashtempl@redhat.com","username":"ashtempl"},"change_message_id":"d568f76f32f8f7600782543d6aaf37cc6c3b7485","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"956d19ea_414a6c89","updated":"2023-10-08 05:41:57.000000000","message":"recheck\n\nRecheck the results","commit_id":"a7a4fcc5060253cfdf897fea644faee50df10d48"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"adc296ddc4e1e11fd19099e2fd56a303a0864696","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"b7359568_ee854903","updated":"2023-10-31 14:50:56.000000000","message":"There are several specific issues with the patch (that I listed in line), but I think a more pressing problem here is that, as per docs you linked to, VLAN provider networks are as good to validate the use case, so why do we need to keep the test VXLAN based?\n\nI think the test could just switch to `provider_net_base_segm_id` as other tests already use for the same goal. Then\n\n1) we don\u0027t need a separate config option just for designate.\n2) we make the test pass in setups that don\u0027t have vxlan configured or supported. (common OVN configuration would be an example of such setup.)\n\n--\n\nLeaving the rest of comments in-line, but most of them are irrelevant once you adopt provider_net_base_segm_id for the test case.","commit_id":"132799fa5e9906d384b72adda7b75e04c36e0006"},{"author":{"_account_id":28609,"name":"Arkady Shtempler","email":"ashtempl@redhat.com","username":"ashtempl"},"change_message_id":"668af62d559bb20aabd6434a88e2812060235507","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"eac92a5e_a8845cc1","updated":"2023-10-30 08:24:29.000000000","message":"recheck\nFailures aren\u0027t related to the change","commit_id":"132799fa5e9906d384b72adda7b75e04c36e0006"}],"neutron_tempest_plugin/config.py":[{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"f6374f7e4a2f2482a23a2d4994c409b20af15e43","unresolved":true,"context_lines":[{"line_number":250,"context_line":"    -cases-3b-and-3c"},{"line_number":251,"context_line":"\"\"\""},{"line_number":252,"context_line":"DnsFeatureGroup \u003d ["},{"line_number":253,"context_line":"    cfg.IntOpt(\u0027segmentation_id\u0027, default\u003d100001,"},{"line_number":254,"context_line":"               help\u003d\"Network segmentation ID\"),"},{"line_number":255,"context_line":"]"},{"line_number":256,"context_line":"dns_feature_group \u003d cfg.OptGroup("}],"source_content_type":"text/x-python","patch_set":5,"id":"ae89e12b_c689235b","line":253,"updated":"2023-10-27 16:00:07.000000000","message":"Ideally, we would have something like CONF.neutron_plugin_options.provider_net_base_segm_id but for vxlan (or geneve), then use it in the test case.\n\nIn this way, the operator would be able to adjust it to their environment as needed. (The 100001 value is not much better than the previous 12345 because both rely on a particular size of tenant network set.)","commit_id":"a7a4fcc5060253cfdf897fea644faee50df10d48"},{"author":{"_account_id":28609,"name":"Arkady Shtempler","email":"ashtempl@redhat.com","username":"ashtempl"},"change_message_id":"ef7e04945491b4bbf73cd2ef229f713421114928","unresolved":true,"context_lines":[{"line_number":250,"context_line":"    -cases-3b-and-3c"},{"line_number":251,"context_line":"\"\"\""},{"line_number":252,"context_line":"DnsFeatureGroup \u003d ["},{"line_number":253,"context_line":"    cfg.IntOpt(\u0027segmentation_id\u0027, default\u003d100001,"},{"line_number":254,"context_line":"               help\u003d\"Network segmentation ID\"),"},{"line_number":255,"context_line":"]"},{"line_number":256,"context_line":"dns_feature_group \u003d cfg.OptGroup("}],"source_content_type":"text/x-python","patch_set":5,"id":"84527d37_ff9b09b2","line":253,"in_reply_to":"8182c156_6856876a","updated":"2023-10-29 15:47:49.000000000","message":"OK, I\u0027ll change it back to 12345","commit_id":"a7a4fcc5060253cfdf897fea644faee50df10d48"},{"author":{"_account_id":28609,"name":"Arkady Shtempler","email":"ashtempl@redhat.com","username":"ashtempl"},"change_message_id":"f0f4d2d2cd97b26a000153a8f3fa0a1206531cd8","unresolved":false,"context_lines":[{"line_number":250,"context_line":"    -cases-3b-and-3c"},{"line_number":251,"context_line":"\"\"\""},{"line_number":252,"context_line":"DnsFeatureGroup \u003d ["},{"line_number":253,"context_line":"    cfg.IntOpt(\u0027segmentation_id\u0027, default\u003d100001,"},{"line_number":254,"context_line":"               help\u003d\"Network segmentation ID\"),"},{"line_number":255,"context_line":"]"},{"line_number":256,"context_line":"dns_feature_group \u003d cfg.OptGroup("}],"source_content_type":"text/x-python","patch_set":5,"id":"6bacc86b_d8951e69","line":253,"in_reply_to":"84527d37_ff9b09b2","updated":"2023-10-31 10:08:17.000000000","message":"Done","commit_id":"a7a4fcc5060253cfdf897fea644faee50df10d48"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"1ac516c4fe7c1dd2471844078b2fb890118d4bd7","unresolved":true,"context_lines":[{"line_number":250,"context_line":"    -cases-3b-and-3c"},{"line_number":251,"context_line":"\"\"\""},{"line_number":252,"context_line":"DnsFeatureGroup \u003d ["},{"line_number":253,"context_line":"    cfg.IntOpt(\u0027segmentation_id\u0027, default\u003d100001,"},{"line_number":254,"context_line":"               help\u003d\"Network segmentation ID\"),"},{"line_number":255,"context_line":"]"},{"line_number":256,"context_line":"dns_feature_group \u003d cfg.OptGroup("}],"source_content_type":"text/x-python","patch_set":5,"id":"8182c156_6856876a","line":253,"in_reply_to":"a86955f6_a7f92bb4","updated":"2023-10-29 04:22:03.000000000","message":"What do you think the range assigned to project networks is in this job? AFAIU it\u0027s 1:2000, so both 12345 and 100001 are out of the \"project\" range.\n\nHere: https://zuul.opendev.org/t/openstack/build/d396d92bd8b445ab827967272825e63b/log/controller/logs/etc/neutron/plugins/ml2/ml2_conf.ini#276\n\n---\n\nIt may be OK for this case. What I\u0027m saying is that in the future, when another test case needs to create an external network, it will have to either introduce another config option for its needs, or migrate this option to a more general approach, as is done with net_base_segm_id for vlans.\n\nIt would be *great* if we could avoid this and implement the same approach as already done for vlans here.","commit_id":"a7a4fcc5060253cfdf897fea644faee50df10d48"},{"author":{"_account_id":28609,"name":"Arkady Shtempler","email":"ashtempl@redhat.com","username":"ashtempl"},"change_message_id":"b03dd1c21b6f5a5ece705585602d3564b9ca8d81","unresolved":true,"context_lines":[{"line_number":250,"context_line":"    -cases-3b-and-3c"},{"line_number":251,"context_line":"\"\"\""},{"line_number":252,"context_line":"DnsFeatureGroup \u003d ["},{"line_number":253,"context_line":"    cfg.IntOpt(\u0027segmentation_id\u0027, default\u003d100001,"},{"line_number":254,"context_line":"               help\u003d\"Network segmentation ID\"),"},{"line_number":255,"context_line":"]"},{"line_number":256,"context_line":"dns_feature_group \u003d cfg.OptGroup("}],"source_content_type":"text/x-python","patch_set":5,"id":"a86955f6_a7f92bb4","line":253,"in_reply_to":"ae89e12b_c689235b","updated":"2023-10-28 08:11:42.000000000","message":"I don\u0027t see any usage of \"segmentation_id\" in neutron tempest plugin code where its value is taken from tempest.conf for testing reason. The only situation it was actually needed is in Designate area therefore I think it\u0027s OK.\n\nCheck commit message title, this is why the default 12345 was changed to 100001\nhttps://review.opendev.org/c/openstack/neutron-tempest-plugin/+/894503#:~:text\u003dREPLY%20(3)-,Set%20segmentation%20ID%20outside%20the%20ranges%20assigned%20to%20project%20network,-For%20use%20cases","commit_id":"a7a4fcc5060253cfdf897fea644faee50df10d48"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"adc296ddc4e1e11fd19099e2fd56a303a0864696","unresolved":true,"context_lines":[{"line_number":245,"context_line":"\"\"\"DNS Integration with an External Service."},{"line_number":246,"context_line":""},{"line_number":247,"context_line":"    :Upstream documentation:"},{"line_number":248,"context_line":"    https://docs.openstack.org/neutron/latest/admin/config-dns-int-ext-"},{"line_number":249,"context_line":"    serv.html#configuration-of-the-externally-accessible-network-for-use"},{"line_number":250,"context_line":"    -cases-3b-and-3c"},{"line_number":251,"context_line":"\"\"\""}],"source_content_type":"text/x-python","patch_set":6,"id":"0359bb0f_1520974d","line":248,"updated":"2023-10-31 14:50:56.000000000","message":"This link refers to a particular scenario. The comment belongs to `segmentation_id` option you add here, not the whole new config section. (Which means that the link belongs to \"before line 253\", if at all needed.)","commit_id":"132799fa5e9906d384b72adda7b75e04c36e0006"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"adc296ddc4e1e11fd19099e2fd56a303a0864696","unresolved":true,"context_lines":[{"line_number":251,"context_line":"\"\"\""},{"line_number":252,"context_line":"DnsFeatureGroup \u003d ["},{"line_number":253,"context_line":"    cfg.IntOpt(\u0027segmentation_id\u0027, default\u003d12345,"},{"line_number":254,"context_line":"               help\u003d\"Network segmentation ID\"),"},{"line_number":255,"context_line":"]"},{"line_number":256,"context_line":"dns_feature_group \u003d cfg.OptGroup("},{"line_number":257,"context_line":"    name\u003d\u0027dns_feature_enabled\u0027, title\u003d\u0027Enabled Designate Features\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"e7f872e8_39726abf","line":254,"updated":"2023-10-31 14:50:56.000000000","message":"The description text doesn\u0027t explain how this ID will be used. (`To create an external network.`) or what the ID is (`it\u0027s VXLAN VNI.` Not e.g. Geneve or GRE.) or how to pick the right value for it. (`it should be outside the range of VXLAN VNIs allocated for project networks.`)","commit_id":"132799fa5e9906d384b72adda7b75e04c36e0006"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"adc296ddc4e1e11fd19099e2fd56a303a0864696","unresolved":true,"context_lines":[{"line_number":254,"context_line":"               help\u003d\"Network segmentation ID\"),"},{"line_number":255,"context_line":"]"},{"line_number":256,"context_line":"dns_feature_group \u003d cfg.OptGroup("},{"line_number":257,"context_line":"    name\u003d\u0027dns_feature_enabled\u0027, title\u003d\u0027Enabled Designate Features\u0027)"},{"line_number":258,"context_line":"CONF.register_group(dns_feature_group)"},{"line_number":259,"context_line":"CONF.register_opts(DnsFeatureGroup, group\u003d\"dns_feature_enabled\")"},{"line_number":260,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"95d07eea_80896616","line":257,"range":{"start_line":257,"start_character":10,"end_line":257,"end_character":29},"updated":"2023-10-31 14:50:56.000000000","message":"Perhaps it\u0027s better to rename the section into something more explicit, e.g. \"designate_feature_enabled\", since \"DNS\" could also be construed to refer to Neutron DNS features.","commit_id":"132799fa5e9906d384b72adda7b75e04c36e0006"}],"neutron_tempest_plugin/scenario/test_dns_integration.py":[{"author":{"_account_id":8313,"name":"Lajos Katona","display_name":"lajoskatona","email":"katonalala@gmail.com","username":"elajkat","status":"Ericsson Software Technology"},"change_message_id":"40836999f7bb375ad85c4e86e7dad0d6a674a420","unresolved":true,"context_lines":[{"line_number":216,"context_line":"        # so that it can be changed to match the deployment if needed"},{"line_number":217,"context_line":"        cls.network2 \u003d cls.create_network(dns_domain\u003dcls.zone[\u0027name\u0027],"},{"line_number":218,"context_line":"                provider_network_type\u003d\u0027vxlan\u0027,"},{"line_number":219,"context_line":"                provider_segmentation_id\u003d12346)"},{"line_number":220,"context_line":"        cls.subnet2 \u003d cls.create_subnet(cls.network2)"},{"line_number":221,"context_line":""},{"line_number":222,"context_line":"    def _verify_dns_assignment(self, port):"}],"source_content_type":"text/x-python","patch_set":2,"id":"880bcbc4_943a3f02","line":219,"updated":"2023-09-14 09:08:20.000000000","message":"Could you follow the TODO to add a new config option for this segmentation id?","commit_id":"827093a22cfc0574f19bf5a51f76530acffc67a3"},{"author":{"_account_id":28609,"name":"Arkady Shtempler","email":"ashtempl@redhat.com","username":"ashtempl"},"change_message_id":"81da9c483febf572603765d6ee63634889174f7b","unresolved":false,"context_lines":[{"line_number":216,"context_line":"        # so that it can be changed to match the deployment if needed"},{"line_number":217,"context_line":"        cls.network2 \u003d cls.create_network(dns_domain\u003dcls.zone[\u0027name\u0027],"},{"line_number":218,"context_line":"                provider_network_type\u003d\u0027vxlan\u0027,"},{"line_number":219,"context_line":"                provider_segmentation_id\u003d12346)"},{"line_number":220,"context_line":"        cls.subnet2 \u003d cls.create_subnet(cls.network2)"},{"line_number":221,"context_line":""},{"line_number":222,"context_line":"    def _verify_dns_assignment(self, port):"}],"source_content_type":"text/x-python","patch_set":2,"id":"8457dde7_031cc8ff","line":219,"in_reply_to":"880bcbc4_943a3f02","updated":"2023-10-02 10:14:52.000000000","message":"Done","commit_id":"827093a22cfc0574f19bf5a51f76530acffc67a3"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"0f99f9a18ce5dbde25a886da743a0dc706ceab03","unresolved":true,"context_lines":[{"line_number":35,"context_line":"LOG \u003d log.getLogger(__name__)"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"# Note(jh): Need to do a bit of juggling here in order to avoid failures"},{"line_number":39,"context_line":"# when designate_tempest_plugin is not available"},{"line_number":40,"context_line":"dns_base \u003d testtools.try_import(\u0027designate_tempest_plugin.tests.base\u0027)"},{"line_number":41,"context_line":"dns_waiters \u003d testtools.try_import(\u0027designate_tempest_plugin.common.waiters\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"1e46dc6c_a90641ac","side":"PARENT","line":38,"updated":"2023-10-27 16:08:48.000000000","message":"the note seems still valid, no?","commit_id":"97409c6992f87f323e135933945ecf8bc03eece3"},{"author":{"_account_id":28609,"name":"Arkady Shtempler","email":"ashtempl@redhat.com","username":"ashtempl"},"change_message_id":"b03dd1c21b6f5a5ece705585602d3564b9ca8d81","unresolved":true,"context_lines":[{"line_number":35,"context_line":"LOG \u003d log.getLogger(__name__)"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"# Note(jh): Need to do a bit of juggling here in order to avoid failures"},{"line_number":39,"context_line":"# when designate_tempest_plugin is not available"},{"line_number":40,"context_line":"dns_base \u003d testtools.try_import(\u0027designate_tempest_plugin.tests.base\u0027)"},{"line_number":41,"context_line":"dns_waiters \u003d testtools.try_import(\u0027designate_tempest_plugin.common.waiters\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"58e4ef5b_ed415aff","side":"PARENT","line":38,"in_reply_to":"1e46dc6c_a90641ac","updated":"2023-10-28 08:11:42.000000000","message":"These tests are using some pieces of code from the \"designate tempest plugin\".\nIf \"designate template plugin\" code is not available it means that the import won\u0027t work for both: dns_base L37 and dns_waiters L38 and if this happens then the error is raised L62, means no tests will be running \u003d \"..avoid failures when designate_tempest_plugin\" is not available.\nThere are no other ways to avoid failures, so yes this note is not relevant.","commit_id":"97409c6992f87f323e135933945ecf8bc03eece3"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"1ac516c4fe7c1dd2471844078b2fb890118d4bd7","unresolved":true,"context_lines":[{"line_number":35,"context_line":"LOG \u003d log.getLogger(__name__)"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"# Note(jh): Need to do a bit of juggling here in order to avoid failures"},{"line_number":39,"context_line":"# when designate_tempest_plugin is not available"},{"line_number":40,"context_line":"dns_base \u003d testtools.try_import(\u0027designate_tempest_plugin.tests.base\u0027)"},{"line_number":41,"context_line":"dns_waiters \u003d testtools.try_import(\u0027designate_tempest_plugin.common.waiters\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"a9fa16a1_08ee2b4e","side":"PARENT","line":38,"in_reply_to":"58e4ef5b_ed415aff","updated":"2023-10-29 04:22:03.000000000","message":"I think the comment tries to explain that try_imports are needed so that the \"whole\" (including non-designate scenarios) tempest run won\u0027t fail if designate plugin is not installed. Which doesn\u0027t change after this patch.","commit_id":"97409c6992f87f323e135933945ecf8bc03eece3"},{"author":{"_account_id":28609,"name":"Arkady Shtempler","email":"ashtempl@redhat.com","username":"ashtempl"},"change_message_id":"ef7e04945491b4bbf73cd2ef229f713421114928","unresolved":true,"context_lines":[{"line_number":35,"context_line":"LOG \u003d log.getLogger(__name__)"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"# Note(jh): Need to do a bit of juggling here in order to avoid failures"},{"line_number":39,"context_line":"# when designate_tempest_plugin is not available"},{"line_number":40,"context_line":"dns_base \u003d testtools.try_import(\u0027designate_tempest_plugin.tests.base\u0027)"},{"line_number":41,"context_line":"dns_waiters \u003d testtools.try_import(\u0027designate_tempest_plugin.common.waiters\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"ed2b2a29_f0a29bb7","side":"PARENT","line":38,"in_reply_to":"a9fa16a1_08ee2b4e","updated":"2023-10-29 15:47:49.000000000","message":"Additional check for \"whole\" designate tempest plugin will be added.","commit_id":"97409c6992f87f323e135933945ecf8bc03eece3"},{"author":{"_account_id":28609,"name":"Arkady Shtempler","email":"ashtempl@redhat.com","username":"ashtempl"},"change_message_id":"f0f4d2d2cd97b26a000153a8f3fa0a1206531cd8","unresolved":false,"context_lines":[{"line_number":35,"context_line":"LOG \u003d log.getLogger(__name__)"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"# Note(jh): Need to do a bit of juggling here in order to avoid failures"},{"line_number":39,"context_line":"# when designate_tempest_plugin is not available"},{"line_number":40,"context_line":"dns_base \u003d testtools.try_import(\u0027designate_tempest_plugin.tests.base\u0027)"},{"line_number":41,"context_line":"dns_waiters \u003d testtools.try_import(\u0027designate_tempest_plugin.common.waiters\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"15ffbb21_689ba8fc","side":"PARENT","line":38,"in_reply_to":"ed2b2a29_f0a29bb7","updated":"2023-10-31 10:08:17.000000000","message":"Done","commit_id":"97409c6992f87f323e135933945ecf8bc03eece3"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"0f99f9a18ce5dbde25a886da743a0dc706ceab03","unresolved":true,"context_lines":[{"line_number":59,"context_line":"                CONF.service_available.designate):"},{"line_number":60,"context_line":"            raise cls.skipException(\"Designate support is required\")"},{"line_number":61,"context_line":"        if not (dns_base and dns_waiters):"},{"line_number":62,"context_line":"            raise cls.skipException("},{"line_number":63,"context_line":"                \"Designate tempest plugin is missing, check if exists on file\""},{"line_number":64,"context_line":"                \" system and added to the PYTHONPATH\")"},{"line_number":65,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"4e147f43_690a1eb1","line":62,"updated":"2023-10-27 16:08:48.000000000","message":"also, I\u0027d prefer if we split these changes and the above from the patch. they are not related.","commit_id":"a7a4fcc5060253cfdf897fea644faee50df10d48"},{"author":{"_account_id":28609,"name":"Arkady Shtempler","email":"ashtempl@redhat.com","username":"ashtempl"},"change_message_id":"f0f4d2d2cd97b26a000153a8f3fa0a1206531cd8","unresolved":false,"context_lines":[{"line_number":59,"context_line":"                CONF.service_available.designate):"},{"line_number":60,"context_line":"            raise cls.skipException(\"Designate support is required\")"},{"line_number":61,"context_line":"        if not (dns_base and dns_waiters):"},{"line_number":62,"context_line":"            raise cls.skipException("},{"line_number":63,"context_line":"                \"Designate tempest plugin is missing, check if exists on file\""},{"line_number":64,"context_line":"                \" system and added to the PYTHONPATH\")"},{"line_number":65,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"e08a237d_8c9b7a44","line":62,"in_reply_to":"3550634e_a7fee6f7","updated":"2023-10-31 10:08:17.000000000","message":"Done","commit_id":"a7a4fcc5060253cfdf897fea644faee50df10d48"},{"author":{"_account_id":28609,"name":"Arkady Shtempler","email":"ashtempl@redhat.com","username":"ashtempl"},"change_message_id":"b03dd1c21b6f5a5ece705585602d3564b9ca8d81","unresolved":true,"context_lines":[{"line_number":59,"context_line":"                CONF.service_available.designate):"},{"line_number":60,"context_line":"            raise cls.skipException(\"Designate support is required\")"},{"line_number":61,"context_line":"        if not (dns_base and dns_waiters):"},{"line_number":62,"context_line":"            raise cls.skipException("},{"line_number":63,"context_line":"                \"Designate tempest plugin is missing, check if exists on file\""},{"line_number":64,"context_line":"                \" system and added to the PYTHONPATH\")"},{"line_number":65,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"de649de5_6ab97990","line":62,"in_reply_to":"4e147f43_690a1eb1","updated":"2023-10-28 08:11:42.000000000","message":"I don\u0027t see any special reason for splinting the patch because of a single exception message that was modified/improved.","commit_id":"a7a4fcc5060253cfdf897fea644faee50df10d48"},{"author":{"_account_id":28609,"name":"Arkady Shtempler","email":"ashtempl@redhat.com","username":"ashtempl"},"change_message_id":"ef7e04945491b4bbf73cd2ef229f713421114928","unresolved":true,"context_lines":[{"line_number":59,"context_line":"                CONF.service_available.designate):"},{"line_number":60,"context_line":"            raise cls.skipException(\"Designate support is required\")"},{"line_number":61,"context_line":"        if not (dns_base and dns_waiters):"},{"line_number":62,"context_line":"            raise cls.skipException("},{"line_number":63,"context_line":"                \"Designate tempest plugin is missing, check if exists on file\""},{"line_number":64,"context_line":"                \" system and added to the PYTHONPATH\")"},{"line_number":65,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3550634e_a7fee6f7","line":62,"in_reply_to":"b7fd9ca4_98d1cb50","updated":"2023-10-29 15:47:49.000000000","message":"I\u0027ll change the skip messages + commit message.","commit_id":"a7a4fcc5060253cfdf897fea644faee50df10d48"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"1ac516c4fe7c1dd2471844078b2fb890118d4bd7","unresolved":true,"context_lines":[{"line_number":59,"context_line":"                CONF.service_available.designate):"},{"line_number":60,"context_line":"            raise cls.skipException(\"Designate support is required\")"},{"line_number":61,"context_line":"        if not (dns_base and dns_waiters):"},{"line_number":62,"context_line":"            raise cls.skipException("},{"line_number":63,"context_line":"                \"Designate tempest plugin is missing, check if exists on file\""},{"line_number":64,"context_line":"                \" system and added to the PYTHONPATH\")"},{"line_number":65,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"b7fd9ca4_98d1cb50","line":62,"in_reply_to":"de649de5_6ab97990","updated":"2023-10-29 04:22:03.000000000","message":"It\u0027s recommended [1] that patches don\u0027t include changes that are not related to the primary topic / commit message. This is why I mentioned it.\n\n(I also don\u0027t think this change to the exception error message makes it much better, but that\u0027s a separate question.)\n\n[1] https://docs.openstack.org/contributors/code-and-documentation/patch-best-practices.html#structural-split-of-changes","commit_id":"a7a4fcc5060253cfdf897fea644faee50df10d48"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"adc296ddc4e1e11fd19099e2fd56a303a0864696","unresolved":true,"context_lines":[{"line_number":59,"context_line":"        if not (\u0027designate\u0027 in CONF.service_available and"},{"line_number":60,"context_line":"                CONF.service_available.designate):"},{"line_number":61,"context_line":"            raise cls.skipException(\"Designate support is required\")"},{"line_number":62,"context_line":"        if not designate_tempest_plugin:"},{"line_number":63,"context_line":"            raise cls.skipException(\"Designate tempest plugin is missing\")"},{"line_number":64,"context_line":"        if not (dns_base and dns_waiters):"},{"line_number":65,"context_line":"            raise cls.skipException("}],"source_content_type":"text/x-python","patch_set":6,"id":"8905c1ed_60a014ab","line":62,"updated":"2023-10-31 14:50:56.000000000","message":"I don\u0027t think this is correct. You should not silently skip the tests when designate is explicitly enabled in tempest config file. (which is guaranteed after lines 59-61.)\n\nSince we know at this point that designate tests are desired (they were enabled in tempest config via service_available option), we should fail to inform the job","commit_id":"132799fa5e9906d384b72adda7b75e04c36e0006"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"adc296ddc4e1e11fd19099e2fd56a303a0864696","unresolved":true,"context_lines":[{"line_number":63,"context_line":"            raise cls.skipException(\"Designate tempest plugin is missing\")"},{"line_number":64,"context_line":"        if not (dns_base and dns_waiters):"},{"line_number":65,"context_line":"            raise cls.skipException("},{"line_number":66,"context_line":"                \"Designate tempest plugin is missing: base.py and waiters.py\")"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    @classmethod"},{"line_number":69,"context_line":"    @utils.requires_ext(extension\u003d\"dns-integration\", service\u003d\"network\")"}],"source_content_type":"text/x-python","patch_set":6,"id":"b5afe7f1_91c8afe8","line":66,"range":{"start_line":66,"start_character":54,"end_line":66,"end_character":76},"updated":"2023-10-31 14:50:56.000000000","message":"I don\u0027t think this edit is helpful, and it\u0027s prone to become obsolete because file names change.","commit_id":"132799fa5e9906d384b72adda7b75e04c36e0006"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"adc296ddc4e1e11fd19099e2fd56a303a0864696","unresolved":true,"context_lines":[{"line_number":215,"context_line":"        super(DNSIntegrationAdminTests, cls).resource_setup()"},{"line_number":216,"context_line":"        cls.network2 \u003d cls.create_network("},{"line_number":217,"context_line":"            dns_domain\u003dcls.zone[\u0027name\u0027],"},{"line_number":218,"context_line":"            provider_network_type\u003d\u0027vxlan\u0027,"},{"line_number":219,"context_line":"            provider_segmentation_id\u003dCONF.dns_feature_enabled.segmentation_id)"},{"line_number":220,"context_line":"        cls.subnet2 \u003d cls.create_subnet(cls.network2)"},{"line_number":221,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"f844088c_c872340e","line":218,"updated":"2023-10-31 14:50:56.000000000","message":"(No action required) This is not your fault, but this \"type\" will also need to be exposed via a config file (in a separate patch) because setups are not required to support vxlan tunnels. (E.g. OVN setups are usually configured for Geneve.)","commit_id":"132799fa5e9906d384b72adda7b75e04c36e0006"}]}