)]}'
{"tempest/config.py":[{"author":{"_account_id":5689,"name":"Masayuki Igawa","email":"masayuki@igawa.io","username":"igawa"},"change_message_id":"9606f6d1aadd03bddc90191dfda065ac401e52c8","unresolved":false,"context_lines":[{"line_number":575,"context_line":"                help\u003d\u0027Does the test environment support attaching a volume to \u0027"},{"line_number":576,"context_line":"                     \u0027more than one instance? This depends on hypervisor and \u0027"},{"line_number":577,"context_line":"                     \u0027volume backend/type and compute API version 2.60.\u0027),"},{"line_number":578,"context_line":"    cfg.BoolOpt(\u0027amd_sev\u0027,"},{"line_number":579,"context_line":"                default\u003dFalse,"},{"line_number":580,"context_line":"                help\u003d\u0027Does the test environment contain AMD compute nodes \u0027"},{"line_number":581,"context_line":"                     \u0027which support SEV (Secure Encrypted Virtualisation)?\u0027),"}],"source_content_type":"text/x-python","patch_set":3,"id":"5faad753_a14d6e73","line":578,"updated":"2019-09-10 07:51:54.000000000","message":"As Felipe mentioned, a reno should be added for this config option addition.\nhttps://docs.openstack.org/tempest/latest/REVIEWING.html#release-notes","commit_id":"d7591e48df31185919cf107dfd8bd0540f3ae1a3"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"2e1543286b053746a586530805385812eacfecd2","unresolved":false,"context_lines":[{"line_number":575,"context_line":"                help\u003d\u0027Does the test environment support attaching a volume to \u0027"},{"line_number":576,"context_line":"                     \u0027more than one instance? This depends on hypervisor and \u0027"},{"line_number":577,"context_line":"                     \u0027volume backend/type and compute API version 2.60.\u0027),"},{"line_number":578,"context_line":"    cfg.BoolOpt(\u0027amd_sev\u0027,"},{"line_number":579,"context_line":"                default\u003dFalse,"},{"line_number":580,"context_line":"                help\u003d\u0027Does the test environment contain AMD compute nodes \u0027"},{"line_number":581,"context_line":"                     \u0027which support SEV (Secure Encrypted Virtualisation)?\u0027),"}],"source_content_type":"text/x-python","patch_set":3,"id":"5faad753_a231ec0d","line":578,"in_reply_to":"5faad753_a14d6e73","updated":"2019-09-10 11:08:58.000000000","message":"Sorry, forgot about that.  Fixed in next patchset.","commit_id":"d7591e48df31185919cf107dfd8bd0540f3ae1a3"}],"tempest/scenario/manager.py":[{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"582af220f76c04bb7d6a7e0ad5d41ad47ecaad27","unresolved":false,"context_lines":[{"line_number":45,"context_line":"class ScenarioTest(tempest.test.BaseTestCase):"},{"line_number":46,"context_line":"    \"\"\"Base class for scenario tests. Uses tempest own clients. \"\"\""},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"    credentials \u003d [\u0027primary\u0027, \u0027admin\u0027]"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"    compute_min_microversion \u003d None"},{"line_number":51,"context_line":"    compute_max_microversion \u003d LATEST_MICROVERSION"}],"source_content_type":"text/x-python","patch_set":1,"id":"5faad753_d2e89917","line":48,"range":{"start_line":48,"start_character":31,"end_line":48,"end_character":36},"updated":"2019-09-09 03:02:10.000000000","message":"This should be done inside the TestServerSEVBasicOps class itself. credentials can be initialized there to be [\u0027primary\u0027, \u0027admin\u0027]. This sort of thing should be done in leaf classes","commit_id":"17446ead221360ff13d338dc59f06e1f5291ebf7"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"2e1543286b053746a586530805385812eacfecd2","unresolved":false,"context_lines":[{"line_number":45,"context_line":"class ScenarioTest(tempest.test.BaseTestCase):"},{"line_number":46,"context_line":"    \"\"\"Base class for scenario tests. Uses tempest own clients. \"\"\""},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"    credentials \u003d [\u0027primary\u0027, \u0027admin\u0027]"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"    compute_min_microversion \u003d None"},{"line_number":51,"context_line":"    compute_max_microversion \u003d LATEST_MICROVERSION"}],"source_content_type":"text/x-python","patch_set":1,"id":"5faad753_34d7874e","line":48,"range":{"start_line":48,"start_character":31,"end_line":48,"end_character":36},"in_reply_to":"5faad753_1c80808b","updated":"2019-09-10 11:08:58.000000000","message":"Actually this is required in order to provide os_admin for setting up the admin clients below.  I don\u0027t understand why it would be better to duplicate admin client setup in individual scenarios, rather than putting it in the base class where it\u0027s reusable.  Happy to hear convincing arguments to the contrary.","commit_id":"17446ead221360ff13d338dc59f06e1f5291ebf7"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"ebbed8933ba2f91519b3be70fcd3c96c972f8114","unresolved":false,"context_lines":[{"line_number":45,"context_line":"class ScenarioTest(tempest.test.BaseTestCase):"},{"line_number":46,"context_line":"    \"\"\"Base class for scenario tests. Uses tempest own clients. \"\"\""},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"    credentials \u003d [\u0027primary\u0027, \u0027admin\u0027]"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"    compute_min_microversion \u003d None"},{"line_number":51,"context_line":"    compute_max_microversion \u003d LATEST_MICROVERSION"}],"source_content_type":"text/x-python","patch_set":1,"id":"5faad753_1c80808b","line":48,"range":{"start_line":48,"start_character":31,"end_line":48,"end_character":36},"in_reply_to":"5faad753_d2e89917","updated":"2019-09-09 13:26:18.000000000","message":"Done","commit_id":"17446ead221360ff13d338dc59f06e1f5291ebf7"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"582af220f76c04bb7d6a7e0ad5d41ad47ecaad27","unresolved":false,"context_lines":[{"line_number":94,"context_line":"    def setup_clients(cls):"},{"line_number":95,"context_line":"        super(ScenarioTest, cls).setup_clients()"},{"line_number":96,"context_line":"        # Clients (in alphabetical order)"},{"line_number":97,"context_line":"        cls.admin_flavors_client \u003d cls.os_admin.flavors_client"},{"line_number":98,"context_line":"        cls.flavors_client \u003d cls.os_primary.flavors_client"},{"line_number":99,"context_line":"        cls.compute_floating_ips_client \u003d ("},{"line_number":100,"context_line":"            cls.os_primary.compute_floating_ips_client)"}],"source_content_type":"text/x-python","patch_set":1,"id":"5faad753_f2ed9525","line":97,"range":{"start_line":97,"start_character":8,"end_line":97,"end_character":62},"updated":"2019-09-09 03:02:10.000000000","message":"Likewise, adding a setup_clients to TestServerSEVBasicOps with these declarations should be done instead.","commit_id":"17446ead221360ff13d338dc59f06e1f5291ebf7"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"ebbed8933ba2f91519b3be70fcd3c96c972f8114","unresolved":false,"context_lines":[{"line_number":94,"context_line":"    def setup_clients(cls):"},{"line_number":95,"context_line":"        super(ScenarioTest, cls).setup_clients()"},{"line_number":96,"context_line":"        # Clients (in alphabetical order)"},{"line_number":97,"context_line":"        cls.admin_flavors_client \u003d cls.os_admin.flavors_client"},{"line_number":98,"context_line":"        cls.flavors_client \u003d cls.os_primary.flavors_client"},{"line_number":99,"context_line":"        cls.compute_floating_ips_client \u003d ("},{"line_number":100,"context_line":"            cls.os_primary.compute_floating_ips_client)"}],"source_content_type":"text/x-python","patch_set":1,"id":"5faad753_5c5fd840","line":97,"range":{"start_line":97,"start_character":8,"end_line":97,"end_character":62},"in_reply_to":"5faad753_f2ed9525","updated":"2019-09-09 13:26:18.000000000","message":"Why have flavors_client in the base class but not admin_flavors_client?  It doesn\u0027t cost anything to have both, and I think it is more convenient for future scenarios to have all the clients immediately available.","commit_id":"17446ead221360ff13d338dc59f06e1f5291ebf7"},{"author":{"_account_id":5689,"name":"Masayuki Igawa","email":"masayuki@igawa.io","username":"igawa"},"change_message_id":"9606f6d1aadd03bddc90191dfda065ac401e52c8","unresolved":false,"context_lines":[{"line_number":45,"context_line":"class ScenarioTest(tempest.test.BaseTestCase):"},{"line_number":46,"context_line":"    \"\"\"Base class for scenario tests. Uses tempest own clients. \"\"\""},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"    credentials \u003d [\u0027primary\u0027, \u0027admin\u0027]"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"    compute_min_microversion \u003d None"},{"line_number":51,"context_line":"    compute_max_microversion \u003d LATEST_MICROVERSION"}],"source_content_type":"text/x-python","patch_set":3,"id":"5faad753_c150aa0d","line":48,"updated":"2019-09-10 07:51:54.000000000","message":"I think you can change test_server_sev.py not this class. \n\nFor example, https://opendev.org/openstack/tempest/src/branch/master/tempest/scenario/test_server_multinode.py#L27","commit_id":"d7591e48df31185919cf107dfd8bd0540f3ae1a3"},{"author":{"_account_id":5689,"name":"Masayuki Igawa","email":"masayuki@igawa.io","username":"igawa"},"change_message_id":"6712503f282b7ae2c13c60133241afe30e8d7848","unresolved":false,"context_lines":[{"line_number":45,"context_line":"class ScenarioTest(tempest.test.BaseTestCase):"},{"line_number":46,"context_line":"    \"\"\"Base class for scenario tests. Uses tempest own clients. \"\"\""},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"    credentials \u003d [\u0027primary\u0027, \u0027admin\u0027]"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"    compute_min_microversion \u003d None"},{"line_number":51,"context_line":"    compute_max_microversion \u003d LATEST_MICROVERSION"}],"source_content_type":"text/x-python","patch_set":3,"id":"5faad753_bc70fedd","line":48,"in_reply_to":"5faad753_666448ba","updated":"2019-09-12 07:27:08.000000000","message":"My concern here is that not all scenario tests need the admin credential.\n\nIf a tester doesn\u0027t have an admin credential, I think all scenario tests would fail even if they should work without admin credential.\n\nDo I miss something?","commit_id":"d7591e48df31185919cf107dfd8bd0540f3ae1a3"},{"author":{"_account_id":5689,"name":"Masayuki Igawa","email":"masayuki@igawa.io","username":"igawa"},"change_message_id":"ed8ab400f9253c0d1e0f12906a0dc3ae24c70b2c","unresolved":false,"context_lines":[{"line_number":45,"context_line":"class ScenarioTest(tempest.test.BaseTestCase):"},{"line_number":46,"context_line":"    \"\"\"Base class for scenario tests. Uses tempest own clients. \"\"\""},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"    credentials \u003d [\u0027primary\u0027, \u0027admin\u0027]"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"    compute_min_microversion \u003d None"},{"line_number":51,"context_line":"    compute_max_microversion \u003d LATEST_MICROVERSION"}],"source_content_type":"text/x-python","patch_set":3,"id":"5faad753_f75a2cf1","line":48,"in_reply_to":"5faad753_76a5f903","updated":"2019-09-11 08:54:18.000000000","message":"Oh, sorry, I missed the discussion.\n\nBut I think we don\u0027t need to expose the admin credential to all subsidiaries at this point unless they need it.\n\nAnd, if we add admin client to another scenario, we can refactor the code. I think your assumption might be true but I also think it\u0027s a kind of refactoring and it should be separated if it can make a patch smaller.","commit_id":"d7591e48df31185919cf107dfd8bd0540f3ae1a3"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"16f728621d37f36ee62acb6289f67262c2568ee9","unresolved":false,"context_lines":[{"line_number":45,"context_line":"class ScenarioTest(tempest.test.BaseTestCase):"},{"line_number":46,"context_line":"    \"\"\"Base class for scenario tests. Uses tempest own clients. \"\"\""},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"    credentials \u003d [\u0027primary\u0027, \u0027admin\u0027]"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"    compute_min_microversion \u003d None"},{"line_number":51,"context_line":"    compute_max_microversion \u003d LATEST_MICROVERSION"}],"source_content_type":"text/x-python","patch_set":3,"id":"5faad753_3b8defd2","line":48,"in_reply_to":"5faad753_bc70fedd","updated":"2019-09-12 11:36:21.000000000","message":"Why would they fail?  They would continue to use the normal non-admin os_primary clients. They would not use the admin clients so I don\u0027t see how they could be affected.  But anyway if you are right, the CI will fail and the V-1 will prevent merging, so you don\u0027t need to worry about that :-)","commit_id":"d7591e48df31185919cf107dfd8bd0540f3ae1a3"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"2e1543286b053746a586530805385812eacfecd2","unresolved":false,"context_lines":[{"line_number":45,"context_line":"class ScenarioTest(tempest.test.BaseTestCase):"},{"line_number":46,"context_line":"    \"\"\"Base class for scenario tests. Uses tempest own clients. \"\"\""},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"    credentials \u003d [\u0027primary\u0027, \u0027admin\u0027]"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"    compute_min_microversion \u003d None"},{"line_number":51,"context_line":"    compute_max_microversion \u003d LATEST_MICROVERSION"}],"source_content_type":"text/x-python","patch_set":3,"id":"5faad753_76a5f903","line":48,"in_reply_to":"5faad753_c150aa0d","updated":"2019-09-10 11:08:58.000000000","message":"This was already discussed in PS1:\n\nhttps://review.opendev.org/#/c/680777/1/tempest/scenario/manager.py@48\n\nhttps://review.opendev.org/#/c/680777/1/tempest/scenario/manager.py@97\n\nI am struggling to understand why it would be better to change the scenario rather than the base class.  If it\u0027s added to test_server_sev.py then as soon as another scenario is added which needs any admin client, there will be code duplication between the two scenarios.  It seems more future-proof to me to put it in the base class along with the setup of all the other clients.  The only reason I can think of for not doing this is if setting up admin credentials is somehow too expensive to be done for all scenarios, but that seems very unlikely.  Can you confirm either way?","commit_id":"d7591e48df31185919cf107dfd8bd0540f3ae1a3"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"bd8270543696a787e56fd17e4c9ab97fc820f3a2","unresolved":false,"context_lines":[{"line_number":45,"context_line":"class ScenarioTest(tempest.test.BaseTestCase):"},{"line_number":46,"context_line":"    \"\"\"Base class for scenario tests. Uses tempest own clients. \"\"\""},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"    credentials \u003d [\u0027primary\u0027, \u0027admin\u0027]"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"    compute_min_microversion \u003d None"},{"line_number":51,"context_line":"    compute_max_microversion \u003d LATEST_MICROVERSION"}],"source_content_type":"text/x-python","patch_set":3,"id":"5faad753_666448ba","line":48,"in_reply_to":"5faad753_f75a2cf1","updated":"2019-09-11 12:27:05.000000000","message":"\u003e Oh, sorry, I missed the discussion. But I think we don\u0027t need to expose the admin credential to all subsidiaries at this point unless they need it.\n\nThanks but I\u0027m still missing an answer to the question \"why should we *not* expose the admin credential?\"\n\nSo far noone has said that it is expensive (e.g. it makes the tests slower, or consumes too much extra RAM).  E.g. does it cause extra API calls or require extra resources?\n\nOTOH I already stated the advantage of exposing it in the base class: this new SEV scenario needs it, and probably others will in the future, so we can avoid extra effort for test writers later on by sharing it now in the base class.\n\nSo far that is one advantage and zero disadvantages.  If I\u0027m missing something then please let me know.\n\n\u003e And, if we add admin client to another scenario, we can refactor the code. I think your assumption might be true but I also think it\u0027s a kind of refactoring and it should be separated if it can make a patch smaller.\n\nOnly 4 lines smaller.  I could separate it but then would I put it before or after the SEV commit?  If I put it after then there is no saving and actually there is more work, because I would have to add the admin credentials to the SEV commit and then immediately move them in the next commit.  If I put it before then it might get rejected due to the lack of clarity explained above, and then I would have to rebase the SEV commit.  I am happy to do that if it is not likely to get rejected, but it seems like a lot of extra complication and I\u0027m not sure there would be much benefit.","commit_id":"d7591e48df31185919cf107dfd8bd0540f3ae1a3"},{"author":{"_account_id":5689,"name":"Masayuki Igawa","email":"masayuki@igawa.io","username":"igawa"},"change_message_id":"b8c3fe71028c7e02667c33036cb97bbc1cc5583e","unresolved":false,"context_lines":[{"line_number":45,"context_line":"class ScenarioTest(tempest.test.BaseTestCase):"},{"line_number":46,"context_line":"    \"\"\"Base class for scenario tests. Uses tempest own clients. \"\"\""},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"    credentials \u003d [\u0027primary\u0027, \u0027admin\u0027]"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"    compute_min_microversion \u003d None"},{"line_number":51,"context_line":"    compute_max_microversion \u003d LATEST_MICROVERSION"}],"source_content_type":"text/x-python","patch_set":5,"id":"5faad753_1ddfb50d","line":48,"updated":"2019-09-13 03:56:31.000000000","message":"I think all scenario tests would be skipped (not failed, I was wrong :-p ) with this change *if test users don\u0027t have their admin credentials* because credentials are checked in skip_checks()[1].\n\n[1] https://opendev.org/openstack/tempest/src/branch/master/tempest/test.py#L287\n\nThe scenario tests weren\u0027t skipped in the gate since we have the admin credential in the setting. However, some of tempest users don\u0027t have their admin credential for thier cloud because they are sometimes production environments.\n\n\nThis is my test log with this change and no admin credential::\n\n-----------------\n$ tempest run --regex tempest.scenario\n{1} setUpClass (tempest.scenario.test_minimum_basic.TestMinimumBasicScenario) ... SKIPPED: Missing Identity Admin API credentials in configuration.\n{1} setUpClass (tempest.scenario.test_network_advanced_server_ops.TestNetworkAdvancedServerOps) ... SKIPPED: Missing Identity Admin API credentials in configuration.\n{1} setUpClass (tempest.scenario.test_security_groups_basic_ops.TestSecurityGroupsBasicOps) ... SKIPPED: Missing Identity Admin API credentials in configuration.\n{1} setUpClass (tempest.scenario.test_server_basic_ops.TestServerBasicOps) ... SKIPPED: Missing Identity Admin API credentials in configuration.\n{1} setUpClass (tempest.scenario.test_volume_backup_restore.TestVolumeBackupRestore) ... SKIPPED: Missing Identity Admin API credentials in configuration.\n{3} setUpClass (tempest.scenario.test_shelve_instance.TestShelveInstance) ... SKIPPED: Missing Identity Admin API credentials in configuration.\n{3} setUpClass (tempest.scenario.test_snapshot_pattern.TestSnapshotPattern) ... SKIPPED: Missing Identity Admin API credentials in configuration.\n{3} setUpClass (tempest.scenario.test_stamp_pattern.TestStampPattern) ... SKIPPED: Missing Identity Admin API credentials in configuration.\n{0} setUpClass (tempest.scenario.test_network_basic_ops.TestNetworkBasicOps) ... SKIPPED: Missing Identity Admin API credentials in configuration.\n{0} setUpClass (tempest.scenario.test_network_v6.TestGettingAddress) ... SKIPPED: Missing Identity Admin API credentials in configuration.\n{0} setUpClass (tempest.scenario.test_server_advanced_ops.TestServerAdvancedOps) ... SKIPPED: Missing Identity Admin API credentials in configuration.\n{0} setUpClass (tempest.scenario.test_server_multinode.TestServerMultinode) ... SKIPPED: Missing Identity Admin API credentials in configuration.\n{0} setUpClass (tempest.scenario.test_volume_boot_pattern.TestVolumeBootPattern) ... SKIPPED: Missing Identity Admin API credentials in configuration.\n{2} setUpClass (tempest.scenario.test_aggregates_basic_ops.TestAggregatesBasicOps) ... SKIPPED: Missing Identity Admin API credentials in configuration.\n{2} setUpClass (tempest.scenario.test_encrypted_cinder_volumes.TestEncryptedCinderVolumes) ... SKIPPED: Missing Identity Admin API credentials in configuration.\n{2} setUpClass (tempest.scenario.test_minbw_allocation_placement.MinBwAllocationPlacementTest) ... SKIPPED: Missing Identity Admin API credentials in configuration.\n{2} setUpClass (tempest.scenario.test_object_storage_basic_ops.TestObjectStorageBasicOps) ... SKIPPED: Missing Identity Admin API credentials in configuration.\n{2} setUpClass (tempest.scenario.test_volume_migrate_attached.TestVolumeMigrateRetypeAttached) ... SKIPPED: Missing Identity Admin API credentials in configuration.\n\n\u003d\u003d\u003d\u003d\u003d\u003d\nTotals\n\u003d\u003d\u003d\u003d\u003d\u003d\nRan: 18 tests in 0.5953 sec.\n - Passed: 0\n - Skipped: 18\n - Expected Fail: 0\n - Unexpected Success: 0\n - Failed: 0\nSum of execute time for each test: 0.0000 sec.\n-----------------\n\nCredentials are checked in skip_checks()\nhttps://opendev.org/openstack/tempest/src/branch/master/tempest/test.py#L287","commit_id":"4efc17746d49f557738fd7aadc2cea4234fe5bbf"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"0693f77f69bf664f59cf94d19a0752290992cd28","unresolved":false,"context_lines":[{"line_number":45,"context_line":"class ScenarioTest(tempest.test.BaseTestCase):"},{"line_number":46,"context_line":"    \"\"\"Base class for scenario tests. Uses tempest own clients. \"\"\""},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"    credentials \u003d [\u0027primary\u0027, \u0027admin\u0027]"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"    compute_min_microversion \u003d None"},{"line_number":51,"context_line":"    compute_max_microversion \u003d LATEST_MICROVERSION"}],"source_content_type":"text/x-python","patch_set":5,"id":"5faad753_e00c6942","line":48,"in_reply_to":"5faad753_1ddfb50d","updated":"2019-09-13 10:38:49.000000000","message":"Ah OK thanks, this is a good reason.  Fixed in PS6.","commit_id":"4efc17746d49f557738fd7aadc2cea4234fe5bbf"}],"tempest/scenario/test_server_sev.py":[{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"582af220f76c04bb7d6a7e0ad5d41ad47ecaad27","unresolved":false,"context_lines":[{"line_number":1,"context_line":"# Copyright 2012 OpenStack Foundation"},{"line_number":2,"context_line":"# All Rights Reserved."},{"line_number":3,"context_line":"#"},{"line_number":4,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\"); you may"}],"source_content_type":"text/x-python","patch_set":1,"id":"5faad753_32bc8d04","line":1,"range":{"start_line":1,"start_character":12,"end_line":1,"end_character":16},"updated":"2019-09-09 03:02:10.000000000","message":"nit: this year is not correct","commit_id":"17446ead221360ff13d338dc59f06e1f5291ebf7"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"ebbed8933ba2f91519b3be70fcd3c96c972f8114","unresolved":false,"context_lines":[{"line_number":1,"context_line":"# Copyright 2012 OpenStack Foundation"},{"line_number":2,"context_line":"# All Rights Reserved."},{"line_number":3,"context_line":"#"},{"line_number":4,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\"); you may"}],"source_content_type":"text/x-python","patch_set":1,"id":"5faad753_dc198888","line":1,"range":{"start_line":1,"start_character":12,"end_line":1,"end_character":16},"in_reply_to":"5faad753_32bc8d04","updated":"2019-09-09 13:26:18.000000000","message":"Good catch, thanks.  Also the organisation should have been SUSE since I added the file.  Fixed in next patch set.","commit_id":"17446ead221360ff13d338dc59f06e1f5291ebf7"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"582af220f76c04bb7d6a7e0ad5d41ad47ecaad27","unresolved":false,"context_lines":[{"line_number":55,"context_line":"    @decorators.idempotent_id(\u0027b11666e1-99da-47d1-9a5c-7800c104be5d\u0027)"},{"line_number":56,"context_line":"    @testtools.skipUnless(CONF.compute_feature_enabled.amd_sev,"},{"line_number":57,"context_line":"                          \u0027AMD SEV is not available.\u0027)"},{"line_number":58,"context_line":"    @decorators.attr(type\u003d\u0027smoke\u0027)"},{"line_number":59,"context_line":"    @utils.services(\u0027compute\u0027, \u0027network\u0027)"},{"line_number":60,"context_line":"    def test_server_basic_ops(self):"},{"line_number":61,"context_line":"        keypair \u003d self.create_keypair()"}],"source_content_type":"text/x-python","patch_set":1,"id":"5faad753_12e63146","line":58,"range":{"start_line":58,"start_character":3,"end_line":58,"end_character":34},"updated":"2019-09-09 03:02:10.000000000","message":"This is a scenario test, not a smoke test. Please see the following: https://docs.openstack.org/tempest/latest/HACKING.html#smoke-attribute","commit_id":"17446ead221360ff13d338dc59f06e1f5291ebf7"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"ebbed8933ba2f91519b3be70fcd3c96c972f8114","unresolved":false,"context_lines":[{"line_number":55,"context_line":"    @decorators.idempotent_id(\u0027b11666e1-99da-47d1-9a5c-7800c104be5d\u0027)"},{"line_number":56,"context_line":"    @testtools.skipUnless(CONF.compute_feature_enabled.amd_sev,"},{"line_number":57,"context_line":"                          \u0027AMD SEV is not available.\u0027)"},{"line_number":58,"context_line":"    @decorators.attr(type\u003d\u0027smoke\u0027)"},{"line_number":59,"context_line":"    @utils.services(\u0027compute\u0027, \u0027network\u0027)"},{"line_number":60,"context_line":"    def test_server_basic_ops(self):"},{"line_number":61,"context_line":"        keypair \u003d self.create_keypair()"}],"source_content_type":"text/x-python","patch_set":1,"id":"5faad753_7738afd2","line":58,"range":{"start_line":58,"start_character":3,"end_line":58,"end_character":34},"in_reply_to":"5faad753_12e63146","updated":"2019-09-09 13:26:18.000000000","message":"I copied this directly from scenario/test_server_basic_ops.py but I see your point as SEV is not core functionality even though this scenario is only testing basic operations.  Removed.","commit_id":"17446ead221360ff13d338dc59f06e1f5291ebf7"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"582af220f76c04bb7d6a7e0ad5d41ad47ecaad27","unresolved":false,"context_lines":[{"line_number":70,"context_line":"        orig_specs \u003d \\"},{"line_number":71,"context_line":"            flavors_client.list_flavor_extra_specs(flavor_id)[\u0027extra_specs\u0027]"},{"line_number":72,"context_line":"        extra_specs \u003d orig_specs.copy()"},{"line_number":73,"context_line":"        extra_specs[\u0027hw:mem_encryption\u0027] \u003d \u0027True\u0027"},{"line_number":74,"context_line":"        self.addCleanup(flavors_client.set_flavor_extra_spec,"},{"line_number":75,"context_line":"                        flavor_id, **orig_specs)"},{"line_number":76,"context_line":"        flavors_client.set_flavor_extra_spec(flavor_id, **extra_specs)"}],"source_content_type":"text/x-python","patch_set":1,"id":"5faad753_d2d17959","line":73,"range":{"start_line":73,"start_character":43,"end_line":73,"end_character":49},"updated":"2019-09-09 03:02:10.000000000","message":"nit: Does this need to be a string? Why not boolean?","commit_id":"17446ead221360ff13d338dc59f06e1f5291ebf7"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"ebbed8933ba2f91519b3be70fcd3c96c972f8114","unresolved":false,"context_lines":[{"line_number":70,"context_line":"        orig_specs \u003d \\"},{"line_number":71,"context_line":"            flavors_client.list_flavor_extra_specs(flavor_id)[\u0027extra_specs\u0027]"},{"line_number":72,"context_line":"        extra_specs \u003d orig_specs.copy()"},{"line_number":73,"context_line":"        extra_specs[\u0027hw:mem_encryption\u0027] \u003d \u0027True\u0027"},{"line_number":74,"context_line":"        self.addCleanup(flavors_client.set_flavor_extra_spec,"},{"line_number":75,"context_line":"                        flavor_id, **orig_specs)"},{"line_number":76,"context_line":"        flavors_client.set_flavor_extra_spec(flavor_id, **extra_specs)"}],"source_content_type":"text/x-python","patch_set":1,"id":"5faad753_b782a74e","line":73,"range":{"start_line":73,"start_character":43,"end_line":73,"end_character":49},"in_reply_to":"5faad753_d2d17959","updated":"2019-09-09 13:26:18.000000000","message":"Yes it does; extra_specs is a DictOfStringsField:\n\nhttps://github.com/openstack/nova/blob/master/nova/objects/flavor.py#L219\n\nso the string \u00271\u0027 would be equally valid here.\n\nextra specs get communicated through nova\u0027s REST API which uses JSON:\n\nhttps://docs.openstack.org/api-ref/compute/?expanded\u003dcreate-extra-specs-for-a-flavor-detail#create-extra-specs-for-a-flavor","commit_id":"17446ead221360ff13d338dc59f06e1f5291ebf7"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"d1c4d04801c1ded3bf18311d7673c4c15e538d2a","unresolved":false,"context_lines":[{"line_number":117,"context_line":"            config_drive\u003dCONF.compute_feature_enabled.config_drive,"},{"line_number":118,"context_line":"            metadata\u003dself.md)"},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"        # FIXME: do verify_ssh etc. just like in test_server_basic_ops.py."},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"        self.assertRaises(lib_exc.Conflict,"},{"line_number":123,"context_line":"                          self.servers_client.suspend_server,"}],"source_content_type":"text/x-python","patch_set":1,"id":"5faad753_7f409c08","line":120,"updated":"2019-09-06 22:04:10.000000000","message":"Here I would like to either reuse the code in test_server_basic_ops.py by inheriting from it, or move that code to manager.Scenario so it can be shared between the two scenarios.\n\nThe first of these two approaches would fall foul of a known bug:\n\nhttp://eavesdrop.openstack.org/irclogs/%23openstack-qa/%23openstack-qa.2019-09-06.log.html#t2019-09-06T14:24:13\n\nso maybe the second is better.  Thoughts?","commit_id":"17446ead221360ff13d338dc59f06e1f5291ebf7"},{"author":{"_account_id":5689,"name":"Masayuki Igawa","email":"masayuki@igawa.io","username":"igawa"},"change_message_id":"be69bc2f40374bdfc76a6f64c4812bd0801ef926","unresolved":false,"context_lines":[{"line_number":117,"context_line":"            config_drive\u003dCONF.compute_feature_enabled.config_drive,"},{"line_number":118,"context_line":"            metadata\u003dself.md)"},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"        # FIXME: do verify_ssh etc. just like in test_server_basic_ops.py."},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"        self.assertRaises(lib_exc.Conflict,"},{"line_number":123,"context_line":"                          self.servers_client.suspend_server,"}],"source_content_type":"text/x-python","patch_set":1,"id":"5faad753_96f0ba69","line":120,"updated":"2019-09-10 07:31:06.000000000","message":"yeah if it\u0027s enough for you.","commit_id":"17446ead221360ff13d338dc59f06e1f5291ebf7"},{"author":{"_account_id":5689,"name":"Masayuki Igawa","email":"masayuki@igawa.io","username":"igawa"},"change_message_id":"69b54218162700d38952c12282598848d8f1557f","unresolved":false,"context_lines":[{"line_number":117,"context_line":"            config_drive\u003dCONF.compute_feature_enabled.config_drive,"},{"line_number":118,"context_line":"            metadata\u003dself.md)"},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"        # FIXME: do verify_ssh etc. just like in test_server_basic_ops.py."},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"        self.assertRaises(lib_exc.Conflict,"},{"line_number":123,"context_line":"                          self.servers_client.suspend_server,"}],"source_content_type":"text/x-python","patch_set":1,"id":"5faad753_32324df3","line":120,"updated":"2019-09-09 02:41:03.000000000","message":"yeah, the second one looks better.","commit_id":"17446ead221360ff13d338dc59f06e1f5291ebf7"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"ebbed8933ba2f91519b3be70fcd3c96c972f8114","unresolved":false,"context_lines":[{"line_number":117,"context_line":"            config_drive\u003dCONF.compute_feature_enabled.config_drive,"},{"line_number":118,"context_line":"            metadata\u003dself.md)"},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"        # FIXME: do verify_ssh etc. just like in test_server_basic_ops.py."},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"        self.assertRaises(lib_exc.Conflict,"},{"line_number":123,"context_line":"                          self.servers_client.suspend_server,"}],"source_content_type":"text/x-python","patch_set":1,"id":"5faad753_f7a49e0a","line":120,"in_reply_to":"5faad753_32324df3","updated":"2019-09-09 13:26:18.000000000","message":"Oh, I just found that manager.py already has check_tenant_network_connectivity():\n\nhttps://github.com/openstack/tempest/blob/master/tempest/scenario/manager.py#L690\n\nShould I just use that instead?","commit_id":"17446ead221360ff13d338dc59f06e1f5291ebf7"},{"author":{"_account_id":5689,"name":"Masayuki Igawa","email":"masayuki@igawa.io","username":"igawa"},"change_message_id":"04319f2d41fb7966a72f5cefb8477b8c235939c0","unresolved":false,"context_lines":[{"line_number":125,"context_line":"            config_drive\u003dCONF.compute_feature_enabled.config_drive,"},{"line_number":126,"context_line":"            metadata\u003dself.md)"},{"line_number":127,"context_line":""},{"line_number":128,"context_line":"        # FIXME: do verify_ssh etc. just like in test_server_basic_ops.py."},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"        self.assertRaises(lib_exc.Conflict,"},{"line_number":131,"context_line":"                          self.servers_client.suspend_server,"}],"source_content_type":"text/x-python","patch_set":6,"id":"3fa7e38b_3a98fe68","line":128,"updated":"2019-09-17 06:58:48.000000000","message":"Do you want to fix this in this patch or another patch? I think we can make it in the following patch.","commit_id":"0386d2218c401fc87d5af016c6a160c7051703f0"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"14edeea30f6c74628a7930d209f24f0aa40b1110","unresolved":false,"context_lines":[{"line_number":125,"context_line":"            config_drive\u003dCONF.compute_feature_enabled.config_drive,"},{"line_number":126,"context_line":"            metadata\u003dself.md)"},{"line_number":127,"context_line":""},{"line_number":128,"context_line":"        # FIXME: do verify_ssh etc. just like in test_server_basic_ops.py."},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"        self.assertRaises(lib_exc.Conflict,"},{"line_number":131,"context_line":"                          self.servers_client.suspend_server,"}],"source_content_type":"text/x-python","patch_set":6,"id":"3fa7e38b_577f5e14","line":128,"in_reply_to":"3fa7e38b_3a98fe68","updated":"2019-09-23 21:56:47.000000000","message":"Yes I could do it in a follow-up.","commit_id":"0386d2218c401fc87d5af016c6a160c7051703f0"}]}