)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":17716,"name":"Luz Cazares","email":"luz.cazares@intel.com","username":"luzcazares"},"change_message_id":"a6b8ecb9b67e4a6607b285f445282052be70c4ec","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Skip when force_tenant_isolation\u003dtrue and no admin credentials"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"AbsoluteLimitsV2Tests requires force_tenant_isolation\u003dTrue,"},{"line_number":10,"context_line":"and if admin credenticals are not present, the testcase will"},{"line_number":11,"context_line":"fail instead of being skipped."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"This is to skip a test when force_tenant_isolation is true and"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"1a1ced50_fe5fa13e","line":10,"range":{"start_line":10,"start_character":13,"end_line":10,"end_character":26},"updated":"2017-03-20 17:36:40.000000000","message":"nit: credentials","commit_id":"23c92393dfc4251c4c9dd3eb7e1d7286ca3cb9b4"},{"author":{"_account_id":20190,"name":"zhufl","email":"zhu.fanglei@zte.com.cn","username":"zhufl"},"change_message_id":"26461952db7b7aebe0632869b8be34f91508fd06","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Skip when force_tenant_isolation\u003dtrue and no admin credentials"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"AbsoluteLimitsV2Tests requires force_tenant_isolation\u003dTrue,"},{"line_number":10,"context_line":"and if admin credenticals are not present, the testcase will"},{"line_number":11,"context_line":"fail instead of being skipped."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"This is to skip a test when force_tenant_isolation is true and"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"1a1ced50_0abb480f","line":10,"range":{"start_line":10,"start_character":13,"end_line":10,"end_character":26},"in_reply_to":"1a1ced50_fe5fa13e","updated":"2017-03-21 01:34:55.000000000","message":"Done","commit_id":"23c92393dfc4251c4c9dd3eb7e1d7286ca3cb9b4"}],"tempest/api/volume/test_volume_absolute_limits.py":[{"author":{"_account_id":5689,"name":"Masayuki Igawa","email":"masayuki@igawa.io","username":"igawa"},"change_message_id":"c379258226f4084b99ab9b1762be8d530a3e55ed","unresolved":false,"context_lines":[{"line_number":21,"context_line":"CONF \u003d config.CONF"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"class AbsoluteLimitsTests(base.BaseVolumeAdminTest):"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    # avoid existing volumes of pre-defined tenant"},{"line_number":27,"context_line":"    force_tenant_isolation \u003d True"}],"source_content_type":"text/x-python","patch_set":10,"id":"5ff73747_7804ee17","line":24,"updated":"2017-04-25 07:22:18.000000000","message":"How about adding a NOTE for this inheritance to avoid confusing?","commit_id":"04540f41a513fde62a9d7418add8cfc9d257a6b3"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"5cf1a48807febc14a5223f34c8231b059937b194","unresolved":false,"context_lines":[{"line_number":24,"context_line":"# NOTE(zhufl): This inherits from BaseVolumeAdminTest because"},{"line_number":25,"context_line":"# it requires force_tenant_isolation\u003dTrue, which need admin"},{"line_number":26,"context_line":"# credentials to create non-admin users for the tests."},{"line_number":27,"context_line":"class AbsoluteLimitsTests(base.BaseVolumeAdminTest):"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"    # avoid existing volumes of pre-defined tenant"},{"line_number":30,"context_line":"    force_tenant_isolation \u003d True"}],"source_content_type":"text/x-python","patch_set":11,"id":"5ff73747_e8747f6a","line":27,"updated":"2017-04-26 08:17:55.000000000","message":"Adding Admin here actually triggers allocation of an extra set of admin\ncredentials which is then not used. But I guess it\u0027s acceptable trade off\nfor simplicity.","commit_id":"ebd5e419d70b8515fbe98d1be865dcbb510a9f5f"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"bbc77c46f67105f919acc04ec5e5cf053742ea79","unresolved":false,"context_lines":[{"line_number":24,"context_line":"# NOTE(zhufl): This inherits from BaseVolumeAdminTest because"},{"line_number":25,"context_line":"# it requires force_tenant_isolation\u003dTrue, which need admin"},{"line_number":26,"context_line":"# credentials to create non-admin users for the tests."},{"line_number":27,"context_line":"class AbsoluteLimitsTests(base.BaseVolumeAdminTest):"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"    # avoid existing volumes of pre-defined tenant"},{"line_number":30,"context_line":"    force_tenant_isolation \u003d True"}],"source_content_type":"text/x-python","patch_set":11,"id":"5ff73747_f7dc7275","line":27,"updated":"2017-04-25 19:15:05.000000000","message":"Why don\u0027t we move this test class under admin path?\nThis test class is not used from the other projects which include Defcore. Then we could move this class safely without any effect.","commit_id":"ebd5e419d70b8515fbe98d1be865dcbb510a9f5f"}],"tempest/common/credentials_factory.py":[{"author":{"_account_id":5803,"name":"Attila Fazekas","email":"afazekas@redhat.com","username":"afazekas"},"change_message_id":"ada5cad9216128de7753c755e9d9ebdee210cf2b","unresolved":false,"context_lines":[{"line_number":76,"context_line":"        try:"},{"line_number":77,"context_line":"            admin_creds \u003d get_configured_admin_credentials("},{"line_number":78,"context_line":"                fill_in\u003dTrue, identity_version\u003didentity_version)"},{"line_number":79,"context_line":"        except Exception:"},{"line_number":80,"context_line":"            raise exceptions.InvalidAdminCredentials()"},{"line_number":81,"context_line":"        return dynamic_creds.DynamicCredentialProvider("},{"line_number":82,"context_line":"            name\u003dname,"}],"source_content_type":"text/x-python","patch_set":8,"id":"dfeb2761_cc13530d","line":79,"updated":"2017-03-31 14:53:58.000000000","message":"we might want to log here something,\nor be more specific about the exception type.","commit_id":"8d40d384bde5e58627605a646208266f46427786"},{"author":{"_account_id":20190,"name":"zhufl","email":"zhu.fanglei@zte.com.cn","username":"zhufl"},"change_message_id":"6921e9e14a3f056ebd890fe315598af8554065ee","unresolved":false,"context_lines":[{"line_number":76,"context_line":"        try:"},{"line_number":77,"context_line":"            admin_creds \u003d get_configured_admin_credentials("},{"line_number":78,"context_line":"                fill_in\u003dTrue, identity_version\u003didentity_version)"},{"line_number":79,"context_line":"        except Exception:"},{"line_number":80,"context_line":"            raise exceptions.InvalidAdminCredentials()"},{"line_number":81,"context_line":"        return dynamic_creds.DynamicCredentialProvider("},{"line_number":82,"context_line":"            name\u003dname,"}],"source_content_type":"text/x-python","patch_set":8,"id":"dfeb2761_5f0599c1","line":79,"in_reply_to":"dfeb2761_cc13530d","updated":"2017-04-01 01:29:10.000000000","message":"yea I will add a log here(it is a bit difficult to have more specific exception type)","commit_id":"8d40d384bde5e58627605a646208266f46427786"}],"tempest/test.py":[{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"06ab1743f4163a4792372b4715a4b96b97be0d41","unresolved":false,"context_lines":[{"line_number":495,"context_line":"                    name\u003dcls.__name__, network_resources\u003dcls.network_resources,"},{"line_number":496,"context_line":"                    force_tenant_isolation\u003dforce_tenant_isolation,"},{"line_number":497,"context_line":"                    identity_version\u003dcls.get_identity_version())"},{"line_number":498,"context_line":"            except lib_exc.InvalidCredentials:"},{"line_number":499,"context_line":"                if force_tenant_isolation:"},{"line_number":500,"context_line":"                    raise cls.skipException("},{"line_number":501,"context_line":"                        \"when using force_tenant_isolation\u003dTrue, \""}],"source_content_type":"text/x-python","patch_set":5,"id":"1a1ced50_0309cf95","line":498,"range":{"start_line":498,"start_character":0,"end_line":498,"end_character":46},"updated":"2017-03-17 11:20:02.000000000","message":"I\u0027m not convinced this is the only exception here.\n\nIf you look at http://git.openstack.org/cgit/openstack/tempest/tree/tempest/common/credentials_factory.py#n76, in case of force_tenant_isolation it will try to read configured admin credentials first. This is done in http://git.openstack.org/cgit/openstack/tempest/tree/tempest/common/credentials_factory.py#n176, and I see there get_credentials may raise InvalidCredentials, but also later on you may get an InvalidConfiguration raised.\n\nThe other issue I see is that this is a bit high in the call stack for intercepting an invalid credentials. Knowing the rest of the code it\u0027s fair to say that only a wrong admin may trigger an InvalidCredentials, but we don\u0027t have a contract on that.\n\nI think the best way would be to catch exceptions around http://git.openstack.org/cgit/openstack/tempest/tree/tempest/common/credentials_factory.py#n76, so we know for sure that something is wrong with configured admin credentials. The logic about what to do with that still belongs in test.py I believe, so we would need a new exception, like InvalidAdminCredentials that could be caught here.\n\nSince we want to make test.py a stable interface for plugins, we\u0027ll need to make the credentials factory stable as well somehow, and it would be good to have good contracts. I\u0027m not quite sure yet how that will look like though :)","commit_id":"22e43a9c93850f0a568795cb7de02e8920a5df6b"},{"author":{"_account_id":17716,"name":"Luz Cazares","email":"luz.cazares@intel.com","username":"luzcazares"},"change_message_id":"a6b8ecb9b67e4a6607b285f445282052be70c4ec","unresolved":false,"context_lines":[{"line_number":494,"context_line":"                    name\u003dcls.__name__, network_resources\u003dcls.network_resources,"},{"line_number":495,"context_line":"                    force_tenant_isolation\u003dforce_tenant_isolation,"},{"line_number":496,"context_line":"                    identity_version\u003dcls.get_identity_version())"},{"line_number":497,"context_line":"            except lib_exc.InvalidAdminCredentials:"},{"line_number":498,"context_line":"                if force_tenant_isolation:"},{"line_number":499,"context_line":"                    raise cls.skipException("},{"line_number":500,"context_line":"                        \"when using force_tenant_isolation\u003dTrue, \""}],"source_content_type":"text/x-python","patch_set":7,"id":"1a1ced50_9ba6ba0e","line":497,"range":{"start_line":497,"start_character":27,"end_line":497,"end_character":50},"updated":"2017-03-20 17:36:40.000000000","message":"At this point no method is raising the new InvalidAdminCredentials exeption.\nStill need to go to credentials factory to throw it.","commit_id":"23c92393dfc4251c4c9dd3eb7e1d7286ca3cb9b4"},{"author":{"_account_id":20190,"name":"zhufl","email":"zhu.fanglei@zte.com.cn","username":"zhufl"},"change_message_id":"26461952db7b7aebe0632869b8be34f91508fd06","unresolved":false,"context_lines":[{"line_number":494,"context_line":"                    name\u003dcls.__name__, network_resources\u003dcls.network_resources,"},{"line_number":495,"context_line":"                    force_tenant_isolation\u003dforce_tenant_isolation,"},{"line_number":496,"context_line":"                    identity_version\u003dcls.get_identity_version())"},{"line_number":497,"context_line":"            except lib_exc.InvalidAdminCredentials:"},{"line_number":498,"context_line":"                if force_tenant_isolation:"},{"line_number":499,"context_line":"                    raise cls.skipException("},{"line_number":500,"context_line":"                        \"when using force_tenant_isolation\u003dTrue, \""}],"source_content_type":"text/x-python","patch_set":7,"id":"1a1ced50_cad93085","line":497,"range":{"start_line":497,"start_character":27,"end_line":497,"end_character":50},"in_reply_to":"1a1ced50_9ba6ba0e","updated":"2017-03-21 01:34:55.000000000","message":"Thanks, I forgot to upload one file:)","commit_id":"23c92393dfc4251c4c9dd3eb7e1d7286ca3cb9b4"},{"author":{"_account_id":17716,"name":"Luz Cazares","email":"luz.cazares@intel.com","username":"luzcazares"},"change_message_id":"a6b8ecb9b67e4a6607b285f445282052be70c4ec","unresolved":false,"context_lines":[{"line_number":499,"context_line":"                    raise cls.skipException("},{"line_number":500,"context_line":"                        \"when using force_tenant_isolation\u003dTrue, \""},{"line_number":501,"context_line":"                        \"admin credentials are needed\")"},{"line_number":502,"context_line":"                else:"},{"line_number":503,"context_line":"                    raise"},{"line_number":504,"context_line":"        return cls._creds_provider"},{"line_number":505,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"1a1ced50_bbe1b600","line":502,"range":{"start_line":502,"start_character":16,"end_line":502,"end_character":21},"updated":"2017-03-20 17:36:40.000000000","message":"else statement can be omitted, just leave the raise statement","commit_id":"23c92393dfc4251c4c9dd3eb7e1d7286ca3cb9b4"},{"author":{"_account_id":20190,"name":"zhufl","email":"zhu.fanglei@zte.com.cn","username":"zhufl"},"change_message_id":"26461952db7b7aebe0632869b8be34f91508fd06","unresolved":false,"context_lines":[{"line_number":499,"context_line":"                    raise cls.skipException("},{"line_number":500,"context_line":"                        \"when using force_tenant_isolation\u003dTrue, \""},{"line_number":501,"context_line":"                        \"admin credentials are needed\")"},{"line_number":502,"context_line":"                else:"},{"line_number":503,"context_line":"                    raise"},{"line_number":504,"context_line":"        return cls._creds_provider"},{"line_number":505,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"1a1ced50_aac5d48e","line":502,"range":{"start_line":502,"start_character":16,"end_line":502,"end_character":21},"in_reply_to":"1a1ced50_bbe1b600","updated":"2017-03-21 01:34:55.000000000","message":"Done","commit_id":"23c92393dfc4251c4c9dd3eb7e1d7286ca3cb9b4"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"cba18c22b1a7d1cba061c535bc9340ffef48b2ff","unresolved":false,"context_lines":[{"line_number":489,"context_line":"                not cls._creds_provider.name \u003d\u003d cls.__name__):"},{"line_number":490,"context_line":"            force_tenant_isolation \u003d getattr(cls, \u0027force_tenant_isolation\u0027,"},{"line_number":491,"context_line":"                                             False)"},{"line_number":492,"context_line":"            try:"},{"line_number":493,"context_line":"                cls._creds_provider \u003d credentials.get_credentials_provider("},{"line_number":494,"context_line":"                    name\u003dcls.__name__, network_resources\u003dcls.network_resources,"},{"line_number":495,"context_line":"                    force_tenant_isolation\u003dforce_tenant_isolation,"},{"line_number":496,"context_line":"                    identity_version\u003dcls.get_identity_version())"},{"line_number":497,"context_line":"            except lib_exc.InvalidAdminCredentials:"},{"line_number":498,"context_line":"                if force_tenant_isolation:"},{"line_number":499,"context_line":"                    raise cls.skipException("},{"line_number":500,"context_line":"                        \"when using force_tenant_isolation\u003dTrue, \""},{"line_number":501,"context_line":"                        \"admin credentials are needed\")"},{"line_number":502,"context_line":"                raise"},{"line_number":503,"context_line":"        return cls._creds_provider"},{"line_number":504,"context_line":""},{"line_number":505,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":8,"id":"1a1ced50_b33e2826","line":502,"range":{"start_line":492,"start_character":0,"end_line":502,"end_character":21},"updated":"2017-03-21 07:15:22.000000000","message":"this might be fine but this is little bit late to skip why do not we do in skip_check like done for admin tests.\n\n@L327 where we have helper (is_admin_available) to check the admin cred instead of relying the other method implementation and their raise exception ?","commit_id":"8d40d384bde5e58627605a646208266f46427786"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"f80bed365ca73a7d23d6600b6d76f9a7c82c13c0","unresolved":false,"context_lines":[{"line_number":489,"context_line":"                not cls._creds_provider.name \u003d\u003d cls.__name__):"},{"line_number":490,"context_line":"            force_tenant_isolation \u003d getattr(cls, \u0027force_tenant_isolation\u0027,"},{"line_number":491,"context_line":"                                             False)"},{"line_number":492,"context_line":"            try:"},{"line_number":493,"context_line":"                cls._creds_provider \u003d credentials.get_credentials_provider("},{"line_number":494,"context_line":"                    name\u003dcls.__name__, network_resources\u003dcls.network_resources,"},{"line_number":495,"context_line":"                    force_tenant_isolation\u003dforce_tenant_isolation,"},{"line_number":496,"context_line":"                    identity_version\u003dcls.get_identity_version())"},{"line_number":497,"context_line":"            except lib_exc.InvalidAdminCredentials:"},{"line_number":498,"context_line":"                if force_tenant_isolation:"},{"line_number":499,"context_line":"                    raise cls.skipException("},{"line_number":500,"context_line":"                        \"when using force_tenant_isolation\u003dTrue, \""},{"line_number":501,"context_line":"                        \"admin credentials are needed\")"},{"line_number":502,"context_line":"                raise"},{"line_number":503,"context_line":"        return cls._creds_provider"},{"line_number":504,"context_line":""},{"line_number":505,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":8,"id":"dfeb2761_7803f243","line":502,"range":{"start_line":492,"start_character":0,"end_line":502,"end_character":21},"in_reply_to":"1a1ced50_5907d317","updated":"2017-03-31 09:09:12.000000000","message":"ah yea, with force_tenant_isolation, it do create the  users and project on fly which need CONF.auth.admin_username","commit_id":"8d40d384bde5e58627605a646208266f46427786"},{"author":{"_account_id":20190,"name":"zhufl","email":"zhu.fanglei@zte.com.cn","username":"zhufl"},"change_message_id":"38fba01936bc0de1c646947c8d7c5780cb05e265","unresolved":false,"context_lines":[{"line_number":489,"context_line":"                not cls._creds_provider.name \u003d\u003d cls.__name__):"},{"line_number":490,"context_line":"            force_tenant_isolation \u003d getattr(cls, \u0027force_tenant_isolation\u0027,"},{"line_number":491,"context_line":"                                             False)"},{"line_number":492,"context_line":"            try:"},{"line_number":493,"context_line":"                cls._creds_provider \u003d credentials.get_credentials_provider("},{"line_number":494,"context_line":"                    name\u003dcls.__name__, network_resources\u003dcls.network_resources,"},{"line_number":495,"context_line":"                    force_tenant_isolation\u003dforce_tenant_isolation,"},{"line_number":496,"context_line":"                    identity_version\u003dcls.get_identity_version())"},{"line_number":497,"context_line":"            except lib_exc.InvalidAdminCredentials:"},{"line_number":498,"context_line":"                if force_tenant_isolation:"},{"line_number":499,"context_line":"                    raise cls.skipException("},{"line_number":500,"context_line":"                        \"when using force_tenant_isolation\u003dTrue, \""},{"line_number":501,"context_line":"                        \"admin credentials are needed\")"},{"line_number":502,"context_line":"                raise"},{"line_number":503,"context_line":"        return cls._creds_provider"},{"line_number":504,"context_line":""},{"line_number":505,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":8,"id":"1a1ced50_5907d317","line":502,"range":{"start_line":492,"start_character":0,"end_line":502,"end_character":21},"in_reply_to":"1a1ced50_b33e2826","updated":"2017-03-21 07:46:12.000000000","message":"Thanks Ghanshyam, if auth.admin_username is not configured but test_accounts_file has admin roles, is_admin_available will\nreturn true but the testcase will still fail with InvalidCredentials. \n(I don\u0027t know what\u0027s the relationship between auth.admin_username and test_accounts_file)","commit_id":"8d40d384bde5e58627605a646208266f46427786"}]}