diff --git a/keystone/common/kvs/core.py b/keystone/common/kvs/core.py index 9f2d35cc87..064825f813 100644 --- a/keystone/common/kvs/core.py +++ b/keystone/common/kvs/core.py @@ -362,8 +362,9 @@ class KeyValueStore(object): @contextlib.contextmanager def _action_with_lock(self, key, lock=None): - """Wrapper context manager to validate and handle the lock and lock - timeout if passed in. + """Wrapper context manager. + + Validates and handles the lock and lock timeout if passed in. """ if not isinstance(lock, KeyValueStoreLock): # NOTE(morganfainberg): Locking only matters if a lock is passed in @@ -385,10 +386,11 @@ class KeyValueStore(object): class KeyValueStoreLock(object): - """Basic KeyValueStoreLock context manager that hooks into the - dogpile.cache backend mutex allowing for distributed locking on resources. + """Basic KeyValueStoreLock context manager. - This is only a write lock, and will not prevent reads from occurring. + Hooks into the dogpile.cache backend mutex allowing for distributed locking + on resources. This is only a write lock, and will not prevent reads from + occurring. """ def __init__(self, mutex, key, locking_enabled=True, lock_timeout=0): @@ -431,7 +433,9 @@ class KeyValueStoreLock(object): def get_key_value_store(name, kvs_region=None): - """Instantiate a new :class:`.KeyValueStore` or return a previous + """Retrieve key value store. + + Instantiate a new :class:`.KeyValueStore` or return a previous instantiation that has the same name. """ global KEY_VALUE_STORE_REGISTRY diff --git a/keystone/common/wsgi.py b/keystone/common/wsgi.py index 0c3ea8e310..f3c476e9fd 100644 --- a/keystone/common/wsgi.py +++ b/keystone/common/wsgi.py @@ -113,8 +113,10 @@ def validate_token_bind(context, token_ref): def best_match_language(req): - """Determines the best available locale from the Accept-Language - HTTP header passed in the request. + """Determines the best available locale. + + This returns best available locale based on the Accept-Language HTTP + header passed in the request. """ if not req.accept_language: return None @@ -328,9 +330,7 @@ class Application(BaseApplication): self.policy_api.enforce(creds, 'admin_required', {}) def _attribute_is_empty(self, ref, attribute): - """Returns true if the attribute in the given ref (which is a - dict) is empty or None. - """ + """Determine if the attribute in ref is empty or None.""" return ref.get(attribute) is None or ref.get(attribute) == '' def _require_attribute(self, ref, attribute): diff --git a/keystone/exception.py b/keystone/exception.py index b5eb7237dd..c6467af108 100644 --- a/keystone/exception.py +++ b/keystone/exception.py @@ -164,8 +164,10 @@ class PKITokenExpected(Error): class SecurityError(Error): - """Avoids exposing details of security failures, unless in insecure_debug - mode. + """Security error exception. + + Avoids exposing details of security errors, unless in insecure_debug mode. + """ amendment = _('(Disable insecure_debug mode to suppress these details.)') @@ -265,9 +267,8 @@ class EndpointNotFound(NotFound): class MetadataNotFound(NotFound): - """(dolph): metadata is not a user-facing concept, - so this exception should not be exposed - """ + # NOTE (dolph): metadata is not a user-facing concept, + # so this exception should not be exposed. message_format = _("An unhandled exception has occurred:" " Could not find metadata.") diff --git a/keystone/federation/core.py b/keystone/federation/core.py index aa6ea8d0c4..a93e3bb5cb 100644 --- a/keystone/federation/core.py +++ b/keystone/federation/core.py @@ -317,8 +317,7 @@ class FederationDriverBase(object): @abc.abstractmethod def get_mapping(self, mapping_id): - """Get a mapping, returns the mapping based - on mapping_id. + """Get a mapping, returns the mapping based on mapping_id. :param mapping_id: id of mapping to get :type mapping_ref: string diff --git a/keystone/federation/utils.py b/keystone/federation/utils.py index 8baf12f9ff..b9dc8f9032 100644 --- a/keystone/federation/utils.py +++ b/keystone/federation/utils.py @@ -446,7 +446,9 @@ class RuleProcessor(object): self.rules = rules def process(self, assertion_data): - """Transform assertion to a dictionary of user name and group ids + """Transform assertion to a dictionary. + + The dictionary contains mapping of user name and group ids based on mapping rules. This function will iterate through the mapping rules to find @@ -689,8 +691,7 @@ class RuleProcessor(object): return new def _verify_all_requirements(self, requirements, assertion): - """Go through the remote requirements of a rule, and compare against - the assertion. + """Compare remote requirements of a rule against the assertion. If a value of ``None`` is returned, the rule with this assertion doesn't apply. diff --git a/keystone/identity/core.py b/keystone/identity/core.py index 6629eaf416..878936c840 100644 --- a/keystone/identity/core.py +++ b/keystone/identity/core.py @@ -1248,6 +1248,7 @@ class IdentityDriverV8(object): @abc.abstractmethod def authenticate(self, user_id, password): """Authenticate a given user and password. + :returns: user_ref :raises AssertionError: If user or password is invalid. """ diff --git a/keystone/oauth1/core.py b/keystone/oauth1/core.py index a72a2c5192..2e52aefe35 100644 --- a/keystone/oauth1/core.py +++ b/keystone/oauth1/core.py @@ -238,8 +238,7 @@ class Oauth1DriverV8(object): @abc.abstractmethod def get_consumer(self, consumer_id): - """Get consumer, returns the consumer id (key) - and description. + """Get consumer, returns the consumer id (key) and description. :param consumer_id: id of consumer to get :type consumer_id: string @@ -250,15 +249,15 @@ class Oauth1DriverV8(object): @abc.abstractmethod def get_consumer_with_secret(self, consumer_id): - """Like get_consumer() but returned consumer_ref includes - the consumer secret. + """Like get_consumer(), but also returns consumer secret. + Returned dictionary consumer_ref includes consumer secret. Secrets should only be shared upon consumer creation; the consumer secret is required to verify incoming OAuth requests. :param consumer_id: id of consumer to get :type consumer_id: string - :returns: consumer_ref + :returns: consumer_ref containing consumer secret """ raise exception.NotImplemented() # pragma: no cover diff --git a/keystone/resource/core.py b/keystone/resource/core.py index 5dcc071aa2..b195fb3390 100644 --- a/keystone/resource/core.py +++ b/keystone/resource/core.py @@ -1017,8 +1017,7 @@ class ResourceDriverBase(object): @abc.abstractmethod def list_projects_in_subtree(self, project_id): - """List all projects in the subtree below the hierarchy of the - given project. + """List all projects in the subtree of a given project. :param project_id: the driver will get the subtree under this project. diff --git a/keystone/tests/unit/common/test_notifications.py b/keystone/tests/unit/common/test_notifications.py index 32da0e344d..9422cc443f 100644 --- a/keystone/tests/unit/common/test_notifications.py +++ b/keystone/tests/unit/common/test_notifications.py @@ -174,8 +174,11 @@ class NotificationsWrapperTestCase(unit.BaseTestCase): class NotificationsTestCase(unit.BaseTestCase): def test_send_notification(self): - """Test the private method _send_notification to ensure event_type, + """Test _send_notification. + + Test the private method _send_notification to ensure event_type, payload, and context are built and passed properly. + """ resource = uuid.uuid4().hex resource_type = EXP_RESOURCE_TYPE @@ -1204,7 +1207,9 @@ class TestCallbackRegistration(unit.BaseTestCase): self.mock_log.logger.getEffectiveLevel.return_value = logging.DEBUG def verify_log_message(self, data): - """Tests that use this are a little brittle because adding more + """Verify log message. + + Tests that use this are a little brittle because adding more logging can break them. TODO(dstanek): remove the need for this in a future refactoring diff --git a/keystone/tests/unit/test_auth.py b/keystone/tests/unit/test_auth.py index 1b7320264a..79b9369437 100644 --- a/keystone/tests/unit/test_auth.py +++ b/keystone/tests/unit/test_auth.py @@ -131,9 +131,7 @@ class AuthBadRequests(AuthTest): context={}, auth={}) def test_empty_remote_user(self): - """Verify that _authenticate_external() raises exception if - REMOTE_USER is set as the empty string. - """ + """Verify exception is raised when REMOTE_USER is an empty string.""" context = {'environment': {'REMOTE_USER': ''}} self.assertRaises( token.controllers.ExternalAuthNotApplicable, @@ -973,8 +971,9 @@ class AuthWithTrust(AuthTest): expires_at="2010-06-04T08:44:31.999999Z") def test_create_trust_without_project_id(self): - """Verify that trust can be created without project id and - token can be generated with that trust. + """Verify that trust can be created without project id. + + Also, token can be generated with that trust. """ unscoped_token = self.get_unscoped_token(self.trustor['name']) context = self._create_auth_context( @@ -1005,9 +1004,7 @@ class AuthWithTrust(AuthTest): self.assertIn(role['id'], role_ids) def test_get_trust_without_auth_context(self): - """Verify that a trust cannot be retrieved when the auth context is - missing. - """ + """Verify a trust cannot be retrieved if auth context is missing.""" unscoped_token = self.get_unscoped_token(self.trustor['name']) context = self._create_auth_context( unscoped_token['access']['token']['id']) diff --git a/keystone/tests/unit/test_backend.py b/keystone/tests/unit/test_backend.py index d6b902b3a2..64ec0265d1 100644 --- a/keystone/tests/unit/test_backend.py +++ b/keystone/tests/unit/test_backend.py @@ -3526,7 +3526,8 @@ class IdentityTests(AssignmentTestHelperMixin): leaf_project['id']) def test_delete_projects_from_ids(self): - """Tests the resource backend call delete_projects_from_ids + """Tests the resource backend call delete_projects_from_ids. + Tests the normal flow of the delete_projects_from_ids backend call, that ensures no project on the list exists after it is succesfully called. @@ -3594,7 +3595,8 @@ class IdentityTests(AssignmentTestHelperMixin): project['id']) def test_delete_large_project_cascade(self): - """Try delete a large project with cascade true + """Try delete a large project with cascade true. + Tree we will create:: +-p1-+ @@ -4695,9 +4697,10 @@ class IdentityTests(AssignmentTestHelperMixin): assert_does_not_contain_names(role_assign_without_names) def test_delete_project_assignments_same_id_as_domain(self): - """Test deleting project assignments in a scenario that - project and domain have the same ID. Only project assignments must - be deleted (i.e USER_PROJECT or GROUP_PROJECT). + """Test deleting project assignments with same project and domain ID. + + Only project assignments must be deleted (i.e USER_PROJECT or + GROUP_PROJECT). Test plan: * Create a project and a domain with the same ID; diff --git a/keystone/tests/unit/test_kvs.py b/keystone/tests/unit/test_kvs.py index 11b75b41f3..a88ee1ac06 100644 --- a/keystone/tests/unit/test_kvs.py +++ b/keystone/tests/unit/test_kvs.py @@ -85,7 +85,9 @@ class RegionProxy2Fixture(proxy.ProxyBackend): class TestMemcacheDriver(api.CacheBackend): - """A test dogpile.cache backend that conforms to the mixin-mechanism for + """A test dogpile.cache backend. + + This test backend conforms to the mixin-mechanism for overriding set and set_multi methods on dogpile memcached drivers. """ diff --git a/keystone/tests/unit/test_middleware.py b/keystone/tests/unit/test_middleware.py index b3bf4d5282..d33e8c001b 100644 --- a/keystone/tests/unit/test_middleware.py +++ b/keystone/tests/unit/test_middleware.py @@ -668,7 +668,8 @@ class AuthContextMiddlewareTest(test_backend_sql.SqlTests, self._assert_tokenless_auth_context(context, ephemeral_user=True) def test_ephemeral_any_user_success(self): - """Ephemeral user does not need a specified user + """Verify ephemeral user does not need a specified user. + Keystone is not looking to match the user, but a corresponding group. """ env = {} @@ -721,7 +722,8 @@ class AuthContextMiddlewareTest(test_backend_sql.SqlTests, extra_environ=env) def test_ephemeral_incorrect_mapping_fail(self): - """Ephemeral user picks up the non-ephemeral user mapping. + """Test ephemeral user picking up the non-ephemeral user mapping. + Looking up the mapping with protocol Id 'x509' will load up the non-ephemeral user mapping, results unauthenticated. """ diff --git a/keystone/tests/unit/test_v2.py b/keystone/tests/unit/test_v2.py index 7b051200a9..70e7d31d26 100644 --- a/keystone/tests/unit/test_v2.py +++ b/keystone/tests/unit/test_v2.py @@ -1259,7 +1259,9 @@ class V2TestCase(RestfulTestCase, CoreApiTests, LegacyV2UsernameTests): return (data, token2) def test_fetch_revocation_list_md5(self): - """If the server is configured for md5, then the revocation list has + """Hash for tokens in revocation list and server config should match. + + If the server is configured for md5, then the revocation list has tokens hashed with MD5. """ # The default hash algorithm is md5. @@ -1270,8 +1272,10 @@ class V2TestCase(RestfulTestCase, CoreApiTests, LegacyV2UsernameTests): self.assertThat(token_hash, matchers.Equals(data['revoked'][0]['id'])) def test_fetch_revocation_list_sha256(self): - """If the server is configured for sha256, then the revocation list has - tokens hashed with SHA256 + """Hash for tokens in revocation list and server config should match. + + If the server is configured for sha256, then the revocation list has + tokens hashed with SHA256. """ hash_algorithm = 'sha256' self.config_fixture.config(group='token', diff --git a/keystone/tests/unit/test_v3_assignment.py b/keystone/tests/unit/test_v3_assignment.py index a6dc08138f..0f0a69c948 100644 --- a/keystone/tests/unit/test_v3_assignment.py +++ b/keystone/tests/unit/test_v3_assignment.py @@ -2575,7 +2575,9 @@ class ImpliedRolesTests(test_v3.RestfulTestCase, test_v3.AssignmentTestMixin, return role def test_root_role_as_implied_role_forbidden(self): - """Create 2 roles that are prohibited from being an implied role. + """Test root role is forbidden to be set as an implied role. + + Create 2 roles that are prohibited from being an implied role. Create 1 additional role which should be accepted as an implied role. Assure the prohibited role names cannot be set as an implied role. Assure the accepted role name which is not a member of the diff --git a/keystone/tests/unit/test_v3_catalog.py b/keystone/tests/unit/test_v3_catalog.py index e4e0396529..2eb9db1462 100644 --- a/keystone/tests/unit/test_v3_catalog.py +++ b/keystone/tests/unit/test_v3_catalog.py @@ -569,8 +569,9 @@ class CatalogTestCase(test_v3.RestfulTestCase): expected_status=http_client.BAD_REQUEST) def test_create_endpoint_with_region(self): - """EndpointV3 creates the region before creating the endpoint, if - endpoint is provided with 'region' and no 'region_id' + """EndpointV3 creates the region before creating the endpoint. + + This occurs when endpoint is provided with 'region' and no 'region_id'. """ ref = unit.new_endpoint_ref_with_region(service_id=self.service_id, region=uuid.uuid4().hex) diff --git a/keystone/tests/unit/test_v3_credential.py b/keystone/tests/unit/test_v3_credential.py index 9a2a9a3ec3..07995f190e 100644 --- a/keystone/tests/unit/test_v3_credential.py +++ b/keystone/tests/unit/test_v3_credential.py @@ -224,7 +224,10 @@ class CredentialTestCase(CredentialBaseTestCase): json.loads(r['blob'])) def test_create_non_ec2_credential(self): - """Call ``POST /credentials`` for creating non-ec2 credential.""" + """Test creating non-ec2 credential. + + Call ``POST /credentials``. + """ blob, ref = unit.new_cert_credential(user_id=self.user['id']) r = self.post('/credentials', body={'credential': ref}) @@ -236,8 +239,9 @@ class CredentialTestCase(CredentialBaseTestCase): r.result['credential']['id']) def test_create_ec2_credential_with_missing_project_id(self): - """Call ``POST /credentials`` for creating ec2 credential with missing - project_id. + """Test Creating ec2 credential with missing project_id. + + Call ``POST /credentials``. """ _, ref = unit.new_ec2_credential(user_id=self.user['id'], project_id=None) @@ -247,8 +251,9 @@ class CredentialTestCase(CredentialBaseTestCase): body={'credential': ref}, expected_status=http_client.BAD_REQUEST) def test_create_ec2_credential_with_invalid_blob(self): - """Call ``POST /credentials`` for creating ec2 credential with invalid - blob. + """Test creating ec2 credential with invalid blob. + + Call ``POST /credentials``. """ ref = unit.new_credential_ref(user_id=self.user['id'], project_id=self.project_id, @@ -287,7 +292,10 @@ class TestCredentialTrustScoped(test_v3.RestfulTestCase): self.config_fixture.config(group='trust', enabled=True) def test_trust_scoped_ec2_credential(self): - """Call ``POST /credentials`` for creating ec2 credential.""" + """Test creating trust scoped ec2 credential. + + Call ``POST /credentials``. + """ # Create the trust ref = unit.new_trust_ref( trustor_user_id=self.user_id, diff --git a/keystone/tests/unit/test_v3_federation.py b/keystone/tests/unit/test_v3_federation.py index a2466e4868..965c99da5f 100644 --- a/keystone/tests/unit/test_v3_federation.py +++ b/keystone/tests/unit/test_v3_federation.py @@ -1236,8 +1236,10 @@ class FederatedIdentityProviderTests(test_v3.RestfulTestCase): self._assign_protocol_to_idp(expected_status=http_client.CREATED) def test_protocol_composite_pk(self): - """Test whether Keystone let's add two entities with identical - names, however attached to different IdPs. + """Test that Keystone can add two entities. + + The entities have identical names, however, attached to different + IdPs. 1. Add IdP and assign it protocol with predefined name 2. Add another IdP and assign it a protocol with same name. diff --git a/keystone/tests/unit/test_v3_resource.py b/keystone/tests/unit/test_v3_resource.py index fc25e7c884..5c234600fd 100644 --- a/keystone/tests/unit/test_v3_resource.py +++ b/keystone/tests/unit/test_v3_resource.py @@ -96,8 +96,9 @@ class ResourceTestCase(test_v3.RestfulTestCase, @test_utils.wip('waiting for projects acting as domains implementation') def test_create_domain_creates_is_domain_project(self): - """Call ``POST /domains`` and check a project that acts as a domain - is created. + """Check a project that acts as a domain is created. + + Call ``POST /domains``. """ # Create a new domain domain_ref = unit.new_domain_ref() @@ -188,8 +189,9 @@ class ResourceTestCase(test_v3.RestfulTestCase, @test_utils.wip('waiting for projects acting as domains implementation') def test_update_domain_updates_is_domain_project(self): - """Call ``PATCH /domains`` and check the project that acts as a domain - is updated. + """Check the project that acts as a domain is updated. + + Call ``PATCH /domains``. """ # Create a new domain domain_ref = unit.new_domain_ref() @@ -375,8 +377,9 @@ class ResourceTestCase(test_v3.RestfulTestCase, @test_utils.wip('waiting for projects acting as domains implementation') def test_delete_domain_deletes_is_domain_project(self): - """Call ``DELETE /domains`` and check the project that acts as a domain - is deleted. + """Check the project that acts as a domain is deleted. + + Call ``DELETE /domains``. """ # Create a new domain domain_ref = unit.new_domain_ref() diff --git a/keystone/token/persistence/backends/kvs.py b/keystone/token/persistence/backends/kvs.py index 99a9e30c2b..3620db58e4 100644 --- a/keystone/token/persistence/backends/kvs.py +++ b/keystone/token/persistence/backends/kvs.py @@ -138,8 +138,10 @@ class Token(token.persistence.TokenDriverV8): return data_copy def _get_user_token_list_with_expiry(self, user_key): - """Return a list of tuples in the format (token_id, token_expiry) for - the user_key. + """Return user token list with token expiry. + + :return: the tuples in the format (token_id, token_expiry) + :rtype: list """ return self._get_key_or_default(user_key, default=[]) diff --git a/keystone/token/provider.py b/keystone/token/provider.py index 3d3b53a25d..da06d91b73 100644 --- a/keystone/token/provider.py +++ b/keystone/token/provider.py @@ -74,8 +74,7 @@ def random_urlsafe_str(): def random_urlsafe_str_to_bytes(s): - """Convert a string generated by :func:`random_urlsafe_str()` to - six.binary_type. + """Convert a string from :func:`random_urlsafe_str()` to six.binary_type. :type s: six.text_type :rtype: six.binary_type diff --git a/keystone/token/providers/common.py b/keystone/token/providers/common.py index 2d125f3944..c9b09cbd47 100644 --- a/keystone/token/providers/common.py +++ b/keystone/token/providers/common.py @@ -180,7 +180,8 @@ class V2TokenDataHelper(object): @classmethod def format_catalog(cls, catalog_ref): - """Munge catalogs from internal to output format + """Munge catalogs from internal to output format. + Internal catalogs look like:: {$REGION: { diff --git a/keystone/trust/core.py b/keystone/trust/core.py index 76a2da6897..43069deb69 100644 --- a/keystone/trust/core.py +++ b/keystone/trust/core.py @@ -236,8 +236,10 @@ class TrustDriverV8(object): @abc.abstractmethod def consume_use(self, trust_id): - """Consume one use when a trust was created with a limitation on its - uses, provided there are still uses available. + """Consume one use of a trust. + + One use of a trust is consumed when the trust was created with a + limitation on its uses, provided there are still uses available. :raises keystone.exception.TrustUseLimitReached: If no remaining uses for trust. diff --git a/tox.ini b/tox.ini index e62d04b25b..a6e39636a4 100644 --- a/tox.ini +++ b/tox.ini @@ -91,7 +91,6 @@ passenv = filename= *.py,keystone-all,keystone-manage show-source = true -# H405: multi line docstring summary not separated with an empty line # D100: Missing docstring in public module # D101: Missing docstring in public class # D102: Missing docstring in public method @@ -103,7 +102,7 @@ show-source = true # D205: Blank line required between one-line summary and description. # D400: First line should end with a period. # D401: First line should be in imperative mood. -ignore = H405,D100,D101,D102,D103,D104,D105,D203,D205,D400,D401 +ignore = D100,D101,D102,D103,D104,D105,D203,D205,D400,D401 exclude=.venv,.git,.tox,build,dist,doc,*openstack/common*,*lib/python*,*egg,tools,vendor,.update-venv,*.ini,*.po,*.pot max-complexity=24