diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 5160d30c..d1119ea8 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -291,7 +291,13 @@ has to be restored by hand. 4. Do not run `ruff format` on it: `permit/api/models.py` is excluded from ruff and typos in `pyproject.toml` and keeps the generator's formatting, so the diff shows only API changes. -5. Run the schema drift check, the offline tests under both pydantic majors (see above) and +5. Add each new model to `__all__` in `permit/__init__.py`, and remove each deleted one: + `from permit import *` binds only the names `__all__` lists, and type checkers treat only + those as exported. `tests/test_fix_permit_exception_deprecation.py` fails until the list + matches. A name the models import for their own use, such as one from `typing`, goes in that + test's `NOT_EXPORTED` instead. + +6. Run the schema drift check, the offline tests under both pydantic majors (see above) and `uv run pre-commit run --all-files`. ### Schema drift check diff --git a/MIGRATION.md b/MIGRATION.md index cb6f919f..680776c2 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -467,13 +467,17 @@ summary. request is sent: ```bash - python -m pytest -W error::DeprecationWarning -W "ignore:Use PermitError instead:DeprecationWarning" + python -m pytest -W error::DeprecationWarning ``` - The second filter is needed in 2.x and 3.x alike: `import permit` warns because - `PermitConnectionError` subclasses the deprecated `PermitException`, and that warning comes from - inside the SDK. To fail on permit's flat methods only, use - `-W "error:permit.api.:DeprecationWarning"`. + It also fails on each line that imports `PermitException`, or reads `permit.PermitException` + or `permit.exceptions.PermitException`, as that line runs (an `except` clause reads it only + when an exception reaches it). A star import of `permit` or `permit.exceptions` binds + `PermitException`, so it fails too, even where the code never uses the name: import the names + the code uses instead. permit 4.0 removes `PermitException`: catch `PermitConnectionError` + instead. permit 2.7.0 to 3.0.0 warn on `import permit` itself ("Use PermitError instead"), + from inside the SDK; on those, add `-W "ignore:Use PermitError instead:DeprecationWarning"`. + To fail on permit's flat methods only, use `-W "error:permit.api.:DeprecationWarning"`. - **To silence them** while you migrate, add filters for the messages: ```ini @@ -482,6 +486,7 @@ summary. filterwarnings = ignore:permit\.api\.\w+\(\) is deprecated:DeprecationWarning ignore:Support for pydantic 1:DeprecationWarning + ignore:PermitException is deprecated:DeprecationWarning ``` In code: `warnings.filterwarnings("ignore", message=r"permit\.api\.\w+\(\) is deprecated", category=DeprecationWarning)`. diff --git a/README.md b/README.md index 0794e613..b23bdc22 100644 --- a/README.md +++ b/README.md @@ -312,6 +312,18 @@ each one issues a `DeprecationWarning` that says what to do instead. query parameter the API has deprecated. Use `permit.api.resource_instances.list_detailed()` instead (see [Detailed lists](#detailed-lists)). Only a call that passes `detailed_key=True` or `detailed_key=False` warns. +- **`PermitException`.** Catch `PermitConnectionError` instead: it is the only exception the + SDK raises that is a `PermitException`. permit 4.0 makes `PermitConnectionError` a direct + subclass of `PermitError`, so handlers of `PermitError` or `PermitConnectionError` keep + working. Importing `PermitException`, or reading `permit.PermitException` or + `permit.exceptions.PermitException`, warns at that line when it runs; later uses of an + imported name do not. Type checkers flag it: mypy with `--enable-error-code deprecated`, + pyright in strict mode. `import permit` does not issue this warning. A star import of + `permit` or `permit.exceptions` binds `PermitException`, so it warns once, at the star-import + line, even where the code never uses the name; importing the names the code uses avoids it. + Until you change the code, the warning filter + `ignore:PermitException is deprecated:DeprecationWarning` silences it. Filters for the + message of earlier releases, "Use PermitError instead", do not match it. By default, Python shows these warnings only when the code that triggers them is in `__main__`, such as the script you run. pytest shows them in its warnings summary. To see diff --git a/permit/__init__.py b/permit/__init__.py index d21a8fb0..661cceda 100644 --- a/permit/__init__.py +++ b/permit/__init__.py @@ -1,37 +1,393 @@ """Permit.io SDK: authorization checks and the Permit REST API from Python. -The `X as X` imports mark the package's public names as explicit re-exports -for type checkers. +`__all__` lists the package's public names: the ones `from permit import *` binds, and the +ones type checkers treat as re-exported. """ +import typing as _typing import warnings as _warnings from permit.api.models import * # noqa: F403 - every API model is part of the public surface -from permit.config import PermitConfig as PermitConfig -from permit.enforcement.enforcer import Action as Action -from permit.enforcement.enforcer import Resource as Resource -from permit.enforcement.enforcer import User as User -from permit.enforcement.interfaces import AssignedRole as AssignedRole -from permit.enforcement.interfaces import AuthorizedUsersResult as AuthorizedUsersResult -from permit.enforcement.interfaces import ResourceInput as ResourceInput -from permit.enforcement.interfaces import TenantDetails as TenantDetails -from permit.enforcement.interfaces import UserInput as UserInput -from permit.exceptions import PermitAlreadyExistsError as PermitAlreadyExistsError -from permit.exceptions import PermitApiDetailedError as PermitApiDetailedError -from permit.exceptions import PermitApiError as PermitApiError -from permit.exceptions import PermitConnectionError as PermitConnectionError -from permit.exceptions import PermitContextChangeError as PermitContextChangeError -from permit.exceptions import PermitContextError as PermitContextError -from permit.exceptions import PermitError as PermitError - -# Deprecated, but still exported for existing callers. -from permit.exceptions import PermitException as PermitException # type: ignore[deprecated] -from permit.exceptions import PermitNotFoundError as PermitNotFoundError -from permit.exceptions import PermitValidationError as PermitValidationError -from permit.permit import Permit as Permit -from permit.utils.context import Context as Context +from permit.config import PermitConfig +from permit.enforcement.enforcer import Action, Resource, User +from permit.enforcement.interfaces import ( + AssignedRole, + AuthorizedUsersResult, + ResourceInput, + TenantDetails, + UserInput, +) +from permit.exceptions import ( + _PERMIT_EXCEPTION_DEPRECATION, + PermitAlreadyExistsError, + PermitApiDetailedError, + PermitApiError, + PermitConnectionError, + PermitContextChangeError, + PermitContextError, + PermitError, + PermitNotFoundError, + PermitValidationError, + _PermitException, +) +from permit.permit import Permit +from permit.utils.context import Context +from permit.utils.deprecation import _warn_deprecated_name from permit.utils.pydantic_version import PYDANTIC_VERSION as _PYDANTIC_VERSION +if _typing.TYPE_CHECKING: + # Deprecated, but still exported for existing callers: __getattr__ below serves it. + from permit.exceptions import PermitException # type: ignore[deprecated] + +# PermitException is served by __getattr__ below, so a star import binds it with its warning. +# tests/test_fix_permit_exception_deprecation.py fails when a public name is missing. +__all__ = [ + "APIHistoryEventFullRead", + "APIHistoryEventRead", + "APIKeyCreate", + "APIKeyOwnerType", + "APIKeyRead", + "APIKeyScopeRead", + "AVPEngineDecisionLog", + "AccessRequestApproved", + "AccessRequestCanceled", + "AccessRequestCreateDetails", + "AccessRequestDenied", + "AccessRequestDetails", + "AccessRequestRead", + "AccessRequestReview", + "AccessRequestReviewDeny", + "AccessRequestUserCreate", + "Action", + "ActionBlockEditable", + "ActionBlockRead", + "ActionObj", + "ActivityDetailsList", + "ActivityDetailsObject", + "ActivityDetailsObjectData", + "ActivityLogEventRead", + "AddRolePermissions", + "ApproveMessage", + "AssignedRole", + "AttributeBlockEditable", + "AttributeBlockRead", + "AttributeType", + "AuditLogModel", + "AuditLogObjectsModel", + "AuditLogReplayRequest", + "AuditLogReplayResponse", + "AuditLogSortKey", + "AuthMechanism", + "AuthorizedUsersResult", + "BillingTierType", + "BulkRoleAssignmentReport", + "BulkRoleUnAssignmentReport", + "ConditionSet", + "ConditionSetCreate", + "ConditionSetRead", + "ConditionSetRuleCreate", + "ConditionSetRuleRead", + "ConditionSetRuleRemove", + "ConditionSetType", + "ConditionSetUpdate", + "Context", + "DataGeneratorLibSchemasSchemaOpalDataConditionSetData", + "DataGeneratorLibSchemasSchemaOpalDataDerivationSettings", + "DataGeneratorLibSchemasSchemaOpalDataDerivedRole", + "DataGeneratorLibSchemasSchemaOpalDataDerivedRoleRule", + "DataGeneratorLibSchemasSchemaOpalDataFullData", + "DataGeneratorLibSchemasSchemaOpalDataResourceInstanceAttributeData", + "DataGeneratorLibSchemasSchemaOpalDataResourceTypeData", + "DataGeneratorLibSchemasSchemaOpalDataRoleData", + "DataGeneratorLibSchemasSchemaOpalDataTenantData", + "DataGeneratorLibSchemasSchemaOpalDataUserData", + "DataSourceConfig", + "DataSourceEntryWithPollingInterval", + "DerivedRoleBlockEdit", + "DerivedRoleBlockRead", + "DerivedRoleRuleCreate", + "DerivedRoleRuleDelete", + "DerivedRoleRuleRead", + "DetailedAuditLogModel", + "DummyEngineModel", + "ElementsConfigCreate", + "ElementsConfigRead", + "ElementsConfigRuntimeRead", + "ElementsConfigUpdate", + "ElementsPermissionLevel", + "ElementsRoleRead", + "ElementsType", + "ElementsUserCreate", + "ElementsUserInviteApprove", + "ElementsUserInviteCreate", + "ElementsUserInviteRead", + "ElementsUserInviteUpdate", + "ElementsUserRoleCreate", + "ElementsUserRoleRemove", + "EmailConfigurationCreate", + "EmailConfigurationRead", + "EmailMessageKeys", + "EmailTemplateMessage", + "EmailTemplateRead", + "EmailTemplateType", + "EmailTemplateUpdate", + "Engine", + "EnvironmentCopy", + "EnvironmentCopyConflictStrategy", + "EnvironmentCopyScope", + "EnvironmentCopyScopeFilters", + "EnvironmentCopyTarget", + "EnvironmentCreate", + "EnvironmentObj", + "EnvironmentRead", + "EnvironmentReadWithEmailConfig", + "EnvironmentStatistics", + "EnvironmentStats", + "EnvironmentUpdate", + "ErrorCode", + "ErrorDetails", + "FailedInvite", + "GenericEngineDecisionLog", + "GroupAddRole", + "GroupAssignUser", + "GroupAssignment", + "GroupCreate", + "GroupRead", + "GroupReadSchema", + "HTTPValidationError", + "HistoricalUsage", + "HttpMethods", + "IdentityRead", + "InviteCreate", + "InviteRead", + "InviteStatus", + "JSONPatchAction", + "JwksConfig", + "JwksObj", + "LimitedPaginatedResultAPIHistoryEventRead", + "LimitedPaginatedResultActivityLogEventRead", + "LimitedPaginatedResultAuditLogModel", + "MailgunEmailConfigurationCreate", + "MailgunEmailConfigurationRead", + "MappingRule", + "MappingRuleUpdate", + "MemberAccessLevel", + "MemberAccessObj", + "Methods", + "MissingUserPolicy", + "MonthlyUsage", + "MultiInviteResult", + "OPAEngineDecisionLog", + "OPALClient", + "OPALCommon", + "OPALHttpFetcherConfig", + "OPALUpdateCallback", + "OPALabels", + "OPAMetrics", + "OnboardingStep", + "OperationApprovalApproved", + "OperationApprovalCanceled", + "OperationApprovalCreateDetails", + "OperationApprovalDenied", + "OperationApprovalDetails", + "OperationApprovalList", + "OperationApprovalRead", + "OperationApprovalReview", + "OperationApprovalUserCreate", + "OrgMemberCreate", + "OrgMemberRead", + "OrgMemberReadWithGrants", + "OrgMemberRemovePermissions", + "OrgMemberUpdate", + "OrganizationCreate", + "OrganizationObj", + "OrganizationRead", + "OrganizationReadWithAPIKey", + "OrganizationStatistics", + "OrganizationStats", + "OrganizationUpdate", + "PDPConfigRead", + "PDPContext", + "PDPDataRefreshRequest", + "PDPDataRefreshResponse", + "PDPShardMigration", + "PaginatedResultAPIHistoryEventRead", + "PaginatedResultAPIKeyRead", + "PaginatedResultAccessRequestRead", + "PaginatedResultActivityLogEventRead", + "PaginatedResultConditionSetRead", + "PaginatedResultElementsConfigRead", + "PaginatedResultElementsUserInviteRead", + "PaginatedResultGroupReadSchema", + "PaginatedResultOperationApprovalList", + "PaginatedResultRelationRead", + "PaginatedResultRelationshipTupleDetailedRead", + "PaginatedResultRelationshipTupleRead", + "PaginatedResultResourceInstanceDetailedRead", + "PaginatedResultResourceInstanceRead", + "PaginatedResultResourceRead", + "PaginatedResultResourceRoleRead", + "PaginatedResultRoleAssignmentDetailedRead", + "PaginatedResultRoleAssignmentRead", + "PaginatedResultRoleRead", + "PaginatedResultTenantRead", + "PaginatedResultUserRead", + "PdpConfigObj", + "PdpValues", + "Permission", + "PermissionLevelRoleRead", + "Permit", + "PermitAlreadyExistsError", + "PermitApiDetailedError", + "PermitApiError", + "PermitBackendSchemasSchemaDerivedRoleRuleDerivationSettings", + "PermitBackendSchemasSchemaOpalDataConditionSetData", + "PermitBackendSchemasSchemaOpalDataDerivationSettings", + "PermitBackendSchemasSchemaOpalDataDerivedRole", + "PermitBackendSchemasSchemaOpalDataDerivedRoleRule", + "PermitBackendSchemasSchemaOpalDataFullData", + "PermitBackendSchemasSchemaOpalDataResourceInstanceAttributeData", + "PermitBackendSchemasSchemaOpalDataResourceTypeData", + "PermitBackendSchemasSchemaOpalDataRoleData", + "PermitBackendSchemasSchemaOpalDataTenantData", + "PermitBackendSchemasSchemaOpalDataUserData", + "PermitConfig", + "PermitConnectionError", + "PermitContextChangeError", + "PermitContextError", + "PermitError", + "PermitException", + "PermitNotFoundError", + "PermitValidationError", + "PolicyGuardRuleCreate", + "PolicyGuardRuleItem", + "PolicyGuardRuleRead", + "PolicyGuardScopeAssociate", + "PolicyGuardScopeCreate", + "PolicyGuardScopeDetail", + "PolicyGuardScopeDetailCreate", + "PolicyGuardScopeRead", + "PolicyRepoCreate", + "PolicyRepoRead", + "PolicyRepoStatus", + "ProjectCreate", + "ProjectObj", + "ProjectRead", + "ProjectUpdate", + "ProxyConfigCreate", + "ProxyConfigRead", + "ProxyConfigUpdate", + "RelationBlockRead", + "RelationCreate", + "RelationRead", + "RelationshipTupleBlockRead", + "RelationshipTupleCreate", + "RelationshipTupleCreateBulkOperation", + "RelationshipTupleCreateBulkOperationResult", + "RelationshipTupleDelete", + "RelationshipTupleDeleteBulkOperation", + "RelationshipTupleDeleteBulkOperationResult", + "RelationshipTupleDetailedRead", + "RelationshipTupleObj", + "RelationshipTupleRead", + "RemoteConfig", + "RemoveRolePermissions", + "RequestStatus", + "RequestType", + "Resource", + "ResourceActionCreate", + "ResourceActionGroupCreate", + "ResourceActionGroupRead", + "ResourceActionGroupUpdate", + "ResourceActionRead", + "ResourceActionUpdate", + "ResourceAttributeCreate", + "ResourceAttributeRead", + "ResourceAttributeUpdate", + "ResourceAttributes", + "ResourceCreate", + "ResourceInput", + "ResourceInstanceBlockRead", + "ResourceInstanceCreate", + "ResourceInstanceCreateBulkOperation", + "ResourceInstanceCreateBulkOperationResult", + "ResourceInstanceDeleteBulkOperation", + "ResourceInstanceDeleteBulkOperationResult", + "ResourceInstanceDetailedRead", + "ResourceInstanceRead", + "ResourceInstanceUpdate", + "ResourceRead", + "ResourceReplace", + "ResourceRoleCreate", + "ResourceRoleList", + "ResourceRoleRead", + "ResourceRoleUpdate", + "ResourceTypeObj", + "ResourceUpdate", + "RoleAssignmentCreate", + "RoleAssignmentDetailedRead", + "RoleAssignmentRead", + "RoleAssignmentRemove", + "RoleAssignmentResourceInstance", + "RoleAssignmentRole", + "RoleAssignmentTenant", + "RoleAssignmentUser", + "RoleBlockEditable", + "RoleCreate", + "RoleCreateBulk", + "RoleCreateBulkOperation", + "RoleCreateBulkOperationResult", + "RoleList", + "RoleRead", + "RoleUpdate", + "SMTPEmailConfigurationCreate", + "SMTPEmailConfigurationRead", + "SSHAuthData", + "SSHAuthDataRead", + "ScopeConfigRead", + "ScopeConfigSet", + "SearchOperator", + "StrippedRelationBlockRead", + "TaskResultEnvironmentRead", + "TaskResultPolicyGuardScopeRead", + "TaskStatus", + "TenantBlockRead", + "TenantCreate", + "TenantCreateBulkOperation", + "TenantCreateBulkOperationResult", + "TenantDeleteBulkOperation", + "TenantDeleteBulkOperationResult", + "TenantDetails", + "TenantObj", + "TenantRead", + "TenantUpdate", + "UsageLimits", + "User", + "UserCreate", + "UserCreateBulkOperation", + "UserCreateBulkOperationResult", + "UserDeleteBulkOperation", + "UserDeleteBulkOperationResult", + "UserInTenant", + "UserInput", + "UserInviteStatus", + "UserObj", + "UserRead", + "UserReplaceBulkOperation", + "UserReplaceBulkOperationResult", + "UserResourceInstanceRole", + "UserRole", + "UserRoleCreate", + "UserRoleRemove", + "UserStatus", + "UserUpdate", + "ValidationError", + "WebhookCreateWithElements", + "WebhookRead", + "WebhookType", + "WebhookUpdate", +] + if _PYDANTIC_VERSION < (2, 0): # Importing any permit module runs this file first, and only once per process, so this # warns once. stacklevel=2 attributes the warning to the code that imported permit (the @@ -43,3 +399,17 @@ DeprecationWarning, stacklevel=2, ) + + +def _getattr(name: str) -> object: + """Serve the deprecated `PermitException`, with its warning, to code that reads it.""" + if name == "PermitException": + _warn_deprecated_name(_PERMIT_EXCEPTION_DEPRECATION) + return _PermitException + msg = f"module {__name__!r} has no attribute {name!r}" + raise AttributeError(msg) + + +if not _typing.TYPE_CHECKING: + # The module __getattr__ (PEP 562), out of type checkers' sight like permit.exceptions' one. + __getattr__ = _getattr diff --git a/permit/exceptions.py b/permit/exceptions.py index 070188ef..bd541b91 100644 --- a/permit/exceptions.py +++ b/permit/exceptions.py @@ -1,5 +1,4 @@ import functools -import warnings from collections.abc import Awaitable, Callable, Coroutine from http import HTTPStatus from typing import TYPE_CHECKING, Any, TypeVar @@ -7,6 +6,7 @@ import aiohttp from typing_extensions import ParamSpec, deprecated +from permit.utils.deprecation import _warn_deprecated_name from permit.utils.pydantic_version import PYDANTIC_VERSION from permit.utils.sdk_logger import sdk_logger @@ -20,6 +20,26 @@ from permit.api.models import ErrorDetails, HTTPValidationError +# What `from permit.exceptions import *` binds. PermitException is served by __getattr__ at +# the end of this module, so a star import binds it with its warning. +__all__ = [ + "DEFAULT_SUPPORT_LINK", + "ErrorDetails", + "HTTPValidationError", + "PermitAlreadyExistsError", + "PermitApiDetailedError", + "PermitApiError", + "PermitConnectionError", + "PermitContextChangeError", + "PermitContextError", + "PermitError", + "PermitException", + "PermitNotFoundError", + "PermitValidationError", + "handle_api_error", + "handle_client_error", +] + DEFAULT_SUPPORT_LINK = "https://permit-io.slack.com/ssb/redirect" P = ParamSpec("P") @@ -30,31 +50,40 @@ class PermitError(Exception): """Permit base exception.""" -@deprecated("Use PermitError instead") +@deprecated( + "PermitException is deprecated and will be removed in permit 4.0; " + "catch PermitConnectionError instead (in 4.0 it becomes a PermitError).", + # Type checkers flag every use. At runtime the marker only records the message: + # __getattr__ below warns instead, whenever code reads the name. + category=None, +) class PermitException(PermitError): # noqa: N818 - public name, kept for existing callers - """Permit base exception (deprecated, use PermitError instead).""" + """Permit base exception (deprecated: catch PermitConnectionError instead).""" -# Subclassing a `@deprecated` class warns (typing_extensions hooks `__init_subclass__`). -# This subclass is the SDK's own, so the warning is silenced here: importing the SDK -# stays warning-free, while code that subclasses or raises `PermitException` still warns. -with warnings.catch_warnings(): - warnings.simplefilter("ignore", DeprecationWarning) +# The SDK refers to PermitException by this name only, so importing it does not warn. The class +# keeps its public name, so its repr, tracebacks and pickles are what they were. +_PermitException = PermitException # type: ignore[deprecated] # the SDK's own reference +# The message the marker above recorded, which type checkers show too. +_PERMIT_EXCEPTION_DEPRECATION: str = vars(_PermitException)["__deprecated__"] +if not TYPE_CHECKING: + # Code that reads `PermitException` gets it from __getattr__, with the warning. + del PermitException - class PermitConnectionError(PermitException): # type: ignore[deprecated] # kept, see docstring - """Permit connection exception. - Note: this deliberately still inherits from the deprecated `PermitException` - rather than from `PermitError`. Re-parenting it looks like tidying, but it - silently breaks every consumer whose handler is `except PermitException` -- - a connection blip would stop being caught and become an unhandled crash. - That is a breaking change worth making, but it belongs in a major version - with a changelog entry, not in a dependency-security patch. - """ +class PermitConnectionError(_PermitException): + """Permit connection exception. + + Note: this deliberately still inherits from the deprecated `PermitException` + rather than from `PermitError`. Re-parenting it looks like tidying, but it + silently breaks every consumer whose handler is `except PermitException` -- + a connection blip would stop being caught and become an unhandled crash. + That breaking change waits for permit 4.0, which removes `PermitException`. + """ - def __init__(self, message: str, *, error: aiohttp.ClientError | None = None) -> None: - super().__init__(message) - self.original_error = error + def __init__(self, message: str, *, error: aiohttp.ClientError | None = None) -> None: + super().__init__(message) + self.original_error = error class PermitContextError(PermitError): @@ -308,3 +337,20 @@ async def wrapped(*args: P.args, **kwargs: P.kwargs) -> R: raise PermitConnectionError(msg, error=err) from err return wrapped + + +def _getattr(name: str) -> object: + """Serve the deprecated `PermitException`, with its warning, to code that reads it.""" + if name == "PermitException": + _warn_deprecated_name(_PERMIT_EXCEPTION_DEPRECATION) + return _PermitException + msg = f"module {__name__!r} has no attribute {name!r}" + raise AttributeError(msg) + + +if not TYPE_CHECKING: + # The module __getattr__ (PEP 562). Type checkers do not see it: to them, a module + # __getattr__ means that every name exists. They see PermitException's declaration. + # dir() does not list the name, because help(), inspect.getmembers() and mock's autospec + # read every name dir() lists, and would warn in code that never names PermitException. + __getattr__ = _getattr diff --git a/permit/utils/deprecation.py b/permit/utils/deprecation.py index 7062fbca..68f405ab 100644 --- a/permit/utils/deprecation.py +++ b/permit/utils/deprecation.py @@ -1,3 +1,4 @@ +import sys from collections.abc import Callable from functools import wraps from inspect import iscoroutinefunction @@ -8,6 +9,32 @@ _F = TypeVar("_F", bound=Callable[..., Any]) +# The names importlib's frozen bootstrap module has: its own until `import importlib` renames it. +_IMPORTLIB_BOOTSTRAP = frozenset({"_frozen_importlib", "importlib._bootstrap"}) + + +def _warn_deprecated_name(message: str) -> None: + """Issue a `DeprecationWarning` attributed to the line that read a deprecated module attribute. + + Call it from a module's ``__getattr__`` (PEP 562) when it serves a deprecated name. + + ``from package import name`` reads the name twice: importlib's ``_handle_fromlist`` first + checks that the package has it, then the importing line reads it. Only the second read + warns, so every access warns once, at the line that made it. + + Args: + message: The warning text, typically naming the replacement. + """ + getattr_frame = sys._getframe(1) # noqa: SLF001 - the documented way to read a caller's frame + reader = getattr_frame.f_back + if ( + reader is not None + and reader.f_code.co_name == "_handle_fromlist" + and reader.f_globals.get("__name__") in _IMPORTLIB_BOOTSTRAP + ): + return + warn(message, DeprecationWarning, stacklevel=3) + def _warn_deprecated(message: str) -> None: """Issue a `DeprecationWarning` attributed to the line that called the caller's caller. diff --git a/pyproject.toml b/pyproject.toml index af954771..246fea78 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -285,6 +285,10 @@ runtime-evaluated-base-classes = ["pydantic.BaseModel", "pydantic.v1.BaseModel"] ] # The SDK's loguru wrapper, and the module that reads loguru's levels for log.level. "permit/{logger,utils/sdk_logger}.py" = ["TID251"] +# __all__ lists the API models, which the package gets from `from permit.api.models import *` +# and ruff cannot follow. tests/test_fix_permit_exception_deprecation.py checks that the +# package serves every name in __all__. +"permit/__init__.py" = ["F405"] # These are standalone CLI programs, not library code: writing the rendered # report to stdout IS their interface, so the "no print" rule does not apply. "{.github/scripts,scripts,skills/permit-python-3-migration/scripts}/*.py" = [ diff --git a/skills/permit-python-3-migration/SKILL.md b/skills/permit-python-3-migration/SKILL.md index 2a27251a..0bb14711 100644 --- a/skills/permit-python-3-migration/SKILL.md +++ b/skills/permit-python-3-migration/SKILL.md @@ -99,12 +99,16 @@ The cases that need a decision most: not trace, such as a client passed in from another module: ```bash - python -m pytest -W error::DeprecationWarning -W "ignore:Use PermitError instead:DeprecationWarning" + python -m pytest -W error::DeprecationWarning ``` - For unittest, pass the same `-W` options to `python -m unittest`. The second filter is - required: `import permit` warns because `PermitConnectionError` subclasses the deprecated - `PermitException`, and without the filter every module that imports permit fails to load. + For unittest, pass the same `-W` option to `python -m unittest`. A line that imports + `PermitException`, or reads it from `permit` or `permit.exceptions`, fails too when it runs: + catch `PermitConnectionError` instead. So does a star import of either module, which binds + `PermitException` and warns once even where the code never uses it: import the names the + code uses instead. If the project resolved + permit 3.0.0, `import permit` itself warns ("Use PermitError instead") and every module that + imports permit fails to load: add `-W "ignore:Use PermitError instead:DeprecationWarning"`. If other libraries' warnings fail the run, use `-W "error:permit.api.:DeprecationWarning"`, which fails only on the flat `permit.api` methods. On pydantic 1 also add `-W "ignore:Support for pydantic 1:DeprecationWarning"` diff --git a/tests/test_fix_permit_exception_deprecation.py b/tests/test_fix_permit_exception_deprecation.py new file mode 100644 index 00000000..a2c5e167 --- /dev/null +++ b/tests/test_fix_permit_exception_deprecation.py @@ -0,0 +1,440 @@ +"""Offline tests for the deprecation of ``PermitException`` (PER-16331). + +permit 4.0 removes ``PermitException``. Until then, every read of the name, from ``permit`` or +from ``permit.exceptions``, issues one DeprecationWarning that names 4.0 and the replacement, +attributed to the line that read it. A star import of either module reads it too: both list it +in ``__all__``, so code that star-imports them keeps the name. Code that neither names it nor +star-imports those modules gets no warning, whatever else it imports. The class itself is +unchanged: ``PermitConnectionError`` still subclasses it, so ``except PermitException`` keeps +catching connection errors. + +This process imported permit before any test ran, so the tests of a first import run in a fresh +interpreter and report every warning recorded there. +""" + +import asyncio +import inspect +import json +import os +import pickle +import pydoc +import subprocess +import sys +import warnings +from pathlib import Path +from types import ModuleType +from typing import Any +from unittest import mock + +import aiohttp +import pytest + +import permit +from permit import exceptions +from permit.exceptions import PermitConnectionError, PermitError, handle_client_error +from permit.utils.deprecation import _warn_deprecated_name +from permit.utils.pydantic_version import PYDANTIC_VERSION + +ON_PYDANTIC_1 = PYDANTIC_VERSION < (2, 0) + +MESSAGE = ( + "PermitException is deprecated and will be removed in permit 4.0; catch " + "PermitConnectionError instead (in 4.0 it becomes a PermitError)." +) + +# The directory that holds the permit package this process imported, so that the fresh +# interpreter imports the same copy whether or not permit is installed. +PERMIT_PARENT = Path(permit.__file__).resolve().parents[1] + +# Each way to read the name, as the first line in a fresh interpreter that mentions permit. +FIRST_READS = [ + "from permit import PermitException", + "from permit.exceptions import PermitException", + "import permit; permit.PermitException", + "import permit.exceptions; permit.exceptions.PermitException", +] + +CONSUMER = """\ +import json +import warnings + +with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + {first_read} + +records = [ + {{ + "category": w.category.__name__, + "message": str(w.message), + "filename": w.filename, + "lineno": w.lineno, + }} + for w in caught +] +print(json.dumps(records)) +""" + +FIRST_READ_LINENO = CONSUMER.splitlines().index(" {first_read}") + 1 + + +def run_in_fresh_interpreter(*arguments: str) -> subprocess.CompletedProcess[str]: + return subprocess.run( + [sys.executable, *arguments], + env={**os.environ, "PYTHONPATH": str(PERMIT_PARENT)}, + capture_output=True, + text=True, + timeout=120, + check=False, + ) + + +def recorded_warnings(caught: list[warnings.WarningMessage]) -> list[tuple[type, str, str, int]]: + return [(w.category, str(w.message), w.filename, w.lineno) for w in caught] + + +def read_permit_exception() -> type[Exception]: + """The class that ``permit.PermitException`` names, read with its warning expected.""" + with pytest.warns(DeprecationWarning, match="PermitException is deprecated"): + return permit.PermitException # type: ignore[deprecated] + + +@pytest.mark.parametrize("first_read", FIRST_READS) +def test_reading_the_name_first_warns_once_at_that_line(tmp_path: Path, first_read: str) -> None: + consumer = tmp_path / "consumer.py" + consumer.write_text(CONSUMER.format(first_read=first_read)) + + result = run_in_fresh_interpreter(str(consumer)) + + assert result.returncode == 0, result.stderr + records: list[dict[str, Any]] = json.loads(result.stdout) + # On pydantic 1, importing permit also warns that pydantic 1 is deprecated, on purpose + # (tests/test_fix_pydantic1_deprecation.py). + records = [record for record in records if "Support for pydantic 1" not in record["message"]] + assert records == [ + { + "category": "DeprecationWarning", + "message": MESSAGE, + "filename": str(consumer), + "lineno": FIRST_READ_LINENO, + } + ] + + +def test_every_read_of_the_name_warns_once_at_its_line(tmp_path: Path) -> None: + reads = [ + "from permit import PermitException", + "from permit.exceptions import PermitException", + "permit.PermitException", + "permit.exceptions.PermitException", + "getattr(permit, 'PermitException')", + "from permit import PermitException", + ] + filename = str(tmp_path / "consumer.py") + code = compile("\n".join(reads), filename, "exec") + + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + exec(code, {"permit": permit}) # noqa: S102 - the reads under test, as a user writes them + + assert recorded_warnings(caught) == [ + (DeprecationWarning, MESSAGE, filename, lineno) for lineno in range(1, len(reads) + 1) + ] + + +# A module __getattr__ that serves a deprecated name, and a function that reads the name. +NAME_READER = """\ +def getattr_hook(): + _warn_deprecated_name("probe message") + + +def {reader}(): + getattr_hook() +""" + +NAME_READER_LINENO = NAME_READER.splitlines().index(" getattr_hook()") + 1 + + +@pytest.mark.parametrize( + ("module_name", "reader", "warnings_issued"), + [ + # importlib's check in `from package import name`, under the names its bootstrap module + # has before and after `import importlib` renames it. + ("_frozen_importlib", "_handle_fromlist", 0), + ("importlib._bootstrap", "_handle_fromlist", 0), + # A function of that name in any other module, or other importlib code, reads the name. + ("user_module", "_handle_fromlist", 1), + ("importlib._bootstrap", "_find_and_load", 1), + ], +) +def test_the_name_warning_skips_only_importlibs_fromlist_check( + tmp_path: Path, module_name: str, reader: str, warnings_issued: int +) -> None: + # warnings skips frames whose file name mentions importlib's bootstrap, so this one doesn't. + filename = str(tmp_path / "reader.py") + namespace: dict[str, Any] = { + "__name__": module_name, + "_warn_deprecated_name": _warn_deprecated_name, + } + code = compile(NAME_READER.format(reader=reader), filename, "exec") + exec(code, namespace) # noqa: S102 - defines the frames under test + + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + namespace[reader]() + + expected = [(DeprecationWarning, "probe message", filename, NAME_READER_LINENO)] + assert recorded_warnings(caught) == expected * warnings_issued + + +@pytest.mark.parametrize( + "code", + [ + "import permit", + "import permit.exceptions", + "import permit.sync", + "from permit import PermitConnectionError, PermitError", + "import permit, permit.exceptions; dir(permit); dir(permit.exceptions)", + "from permit import PermitConnectionError\nclass Mine(PermitConnectionError): pass", + ], +) +def test_code_that_does_not_name_it_does_not_warn(code: str) -> None: + options = ["-W", "error"] + if ON_PYDANTIC_1: + # importing permit warns on pydantic 1 on purpose; the later -W option takes precedence. + options += ["-W", "ignore:Support for pydantic 1 is deprecated:DeprecationWarning"] + + result = run_in_fresh_interpreter(*options, "-c", code) + + assert result.returncode == 0, result.stderr + assert result.stderr == "" + + +# A star import, then a handler for PermitException, as code written for permit 2.x has them. +# The prelude imports one of the clients first, or nothing. +STAR_IMPORTER = """\ +import json +import warnings +{prelude} + +with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + from {module} import * + +try: + raise PermitConnectionError("boom") +except PermitException as error: + handled = type(error).__name__ + +records = [ + {{ + "category": w.category.__name__, + "message": str(w.message), + "filename": w.filename, + "lineno": w.lineno, + }} + for w in caught +] +print(json.dumps({{"warnings": records, "handled": handled}})) +""" + +STAR_IMPORT_LINENO = STAR_IMPORTER.splitlines().index(" from {module} import *") + 1 + + +@pytest.mark.parametrize("module", ["permit", "permit.exceptions"]) +@pytest.mark.parametrize( + "prelude", + ["", "from permit import Permit", "from permit.sync import Permit"], + ids=["star-import-first", "async-client-first", "sync-client-first"], +) +def test_a_star_import_binds_the_name_and_warns_once_at_its_line( + tmp_path: Path, module: str, prelude: str +) -> None: + consumer = tmp_path / "consumer.py" + consumer.write_text(STAR_IMPORTER.format(prelude=prelude, module=module)) + + result = run_in_fresh_interpreter(str(consumer)) + + assert result.returncode == 0, result.stderr + output: dict[str, Any] = json.loads(result.stdout) + # On pydantic 1, importing permit also warns that pydantic 1 is deprecated, on purpose. + records = [ + record for record in output["warnings"] if "Support for pydantic 1" not in record["message"] + ] + assert records == [ + { + "category": "DeprecationWarning", + "message": MESSAGE, + "filename": str(consumer), + "lineno": STAR_IMPORT_LINENO, + } + ] + assert output["handled"] == "PermitConnectionError" + + +# Names a star import bound before the modules had __all__ and binds no longer: names the +# modules import for their own use from the standard library, typing, pydantic and aiohttp, and +# permit.exceptions' type variables and its imports of two SDK internals. +NOT_EXPORTED = { + # What `from permit.api.models import *` brings into permit besides the models. + "permit": { + "Any", + "AnyUrl", + "BaseModel", + "Dict", + "EmailStr", + "Enum", + "Extra", + "Field", + "List", + "Literal", + "Optional", + "UUID", + "Union", + "annotations", + "conint", + "constr", + "datetime", + }, + "permit.exceptions": { + "Any", + "Awaitable", + "Callable", + "Coroutine", + "HTTPStatus", + "P", + "PYDANTIC_VERSION", + "ParamSpec", + "R", + "TYPE_CHECKING", + "TypeVar", + "ValidationError", + "aiohttp", + "deprecated", + "functools", + "sdk_logger", + }, +} + + +def public_names(module: ModuleType) -> set[str]: + """The names the module binds without a leading underscore, other than its submodules. + + Importing a submodule binds it in its package, so which ones ``permit`` has depends on what + the process imported: ``sync`` only after ``import permit.sync``. A star import binds none. + """ + return { + name + for name, value in vars(module).items() + if not name.startswith("_") + and not (isinstance(value, ModuleType) and value.__name__ == f"{module.__name__}.{name}") + } + + +@pytest.mark.parametrize("module", [permit, exceptions], ids=["permit", "permit.exceptions"]) +def test_all_lists_every_public_name(module: ModuleType) -> None: + """A star import binds only what __all__ lists, so a public name missing from it is lost.""" + listed: list[str] = module.__all__ + public = public_names(module) + not_exported = NOT_EXPORTED[module.__name__] + + missing = sorted(public - not_exported - set(listed)) + assert missing == [], f"Add these to {module.__name__}.__all__ (or to NOT_EXPORTED)" + assert sorted(not_exported - public) == [], "The module no longer binds these: drop them" + assert sorted(not_exported & set(listed)) == [] + # A star import reads each listed name, so a duplicate PermitException would warn twice. + assert len(set(listed)) == len(listed), f"{module.__name__}.__all__ lists a name twice" + + +@pytest.mark.parametrize("module", [permit, exceptions], ids=["permit", "permit.exceptions"]) +def test_all_lists_only_names_the_module_serves(module: ModuleType) -> None: + """Every listed name is an attribute, except PermitException, which __getattr__ serves.""" + listed: list[str] = module.__all__ + + assert sorted(set(listed) - public_names(module)) == ["PermitException"] + + +@pytest.mark.parametrize("module", [permit, exceptions], ids=["permit", "permit.exceptions"]) +def test_introspecting_a_module_does_not_warn(module: ModuleType) -> None: + """help(), inspect.getmembers() and mock's autospec read every name that dir() lists.""" + with warnings.catch_warnings(): + warnings.simplefilter("error") + pydoc.render_doc(module) + inspect.getmembers(module) + mock.create_autospec(module) + + +def test_both_modules_serve_the_same_class() -> None: + with pytest.warns(DeprecationWarning, match="PermitException is deprecated"): + from_exceptions = exceptions.PermitException # type: ignore[deprecated] + + assert read_permit_exception() is from_exceptions + assert PermitConnectionError.__mro__[1] is from_exceptions + + +def test_except_permit_exception_still_catches_a_connection_error() -> None: + """Regression guard, not an endorsement: 4.0 re-parents PermitConnectionError, not 3.x. + + Consumers of 2.x catch connection failures with ``except PermitException``. Moving + PermitConnectionError under PermitError in 3.x would silently stop that handler from + catching them. + """ + permit_exception = read_permit_exception() + + @handle_client_error + async def send() -> None: + raise aiohttp.ClientConnectionError + + async def call_as_existing_code_does() -> str: + try: + await send() + except permit_exception: + return "caught" + return "not raised" + + assert asyncio.run(call_as_existing_code_does()) == "caught" + + +def test_the_class_and_its_subclass_are_unchanged() -> None: + permit_exception = read_permit_exception() + error = PermitConnectionError("boom") + + assert isinstance(error, permit_exception) + assert isinstance(error, PermitError) + assert [f"{cls.__module__}.{cls.__qualname__}" for cls in PermitConnectionError.__mro__] == [ + "permit.exceptions.PermitConnectionError", + "permit.exceptions.PermitException", + "permit.exceptions.PermitError", + "builtins.Exception", + "builtins.BaseException", + "builtins.object", + ] + assert repr(permit_exception("boom")) == "PermitException('boom')" + assert repr(error) == "PermitConnectionError('boom')" + + +def test_a_connection_error_pickles_without_a_warning() -> None: + error = PermitConnectionError("boom") + + with warnings.catch_warnings(): + warnings.simplefilter("error") + restored = pickle.loads(pickle.dumps(error)) # noqa: S301 - this process pickled it + + assert type(restored) is PermitConnectionError + assert restored.args == ("boom",) + assert restored.original_error is None + + +def test_only_reading_the_name_warns() -> None: + """Creating, raising, catching or subclassing the class does not warn by itself. + + ``_PermitException`` is the name the SDK itself uses for the class. + """ + with warnings.catch_warnings(): + warnings.simplefilter("error") + with pytest.raises(exceptions._PermitException): + raise exceptions._PermitException + + class Custom(exceptions._PermitException): + pass + + Custom("boom") diff --git a/tests/test_fix_pydantic1_deprecation.py b/tests/test_fix_pydantic1_deprecation.py index a3aa910d..052ba92b 100644 --- a/tests/test_fix_pydantic1_deprecation.py +++ b/tests/test_fix_pydantic1_deprecation.py @@ -106,11 +106,13 @@ def test_importing_permit_on_pydantic_2_does_not_warn(tmp_path: Path, first_impo def test_the_pydantic_version_permit_checks_is_not_a_public_name() -> None: """Permit reads the pydantic version to decide whether to warn; the constant is not API. - permit has no ``__all__``, so any name without a leading underscore is public: it is in - ``dir(permit)`` and ``from permit import *`` exports it. + A name without a leading underscore is in ``dir(permit)``, and one in ``permit.__all__`` is + what ``from permit import *`` exports. """ exported: dict[str, object] = {} - exec("from permit import *", exported) # noqa: S102 - what a star import exports is the subject + # The star import also binds the deprecated PermitException, which warns. + with pytest.warns(DeprecationWarning, match="PermitException is deprecated"): + exec("from permit import *", exported) # noqa: S102 - the star import is the subject assert "PYDANTIC_VERSION" not in exported, "from permit import * exports PYDANTIC_VERSION" assert not hasattr(permit, "PYDANTIC_VERSION") diff --git a/tests/test_offline_regressions.py b/tests/test_offline_regressions.py index 536aab09..0afd569a 100644 --- a/tests/test_offline_regressions.py +++ b/tests/test_offline_regressions.py @@ -8,7 +8,6 @@ import ast import inspect -import subprocess import sys import warnings from collections.abc import AsyncIterator, Sequence @@ -29,7 +28,7 @@ from werkzeug import Request import permit -from permit import Permit, Resource, User, exceptions +from permit import Permit, Resource, User from permit.api.context import ApiKeyAccessLevel from permit.api.encoders import jsonable_encoder from permit.api.environments import EnvironmentsApi @@ -510,14 +509,6 @@ async def test_handle_api_error_rejects_redirect_statuses( assert exc_info.value.status_code == status -def test_permit_connection_error_still_caught_by_the_deprecated_base() -> None: - # Regression guard, not an endorsement. `PermitException` is deprecated, - # but consumers on 2.6.x catch it, and re-parenting PermitConnectionError - # onto PermitError would silently stop `except PermitException` from - # catching connection failures. Re-parent it in a major version, not here. - assert issubclass(PermitConnectionError, exceptions.PermitException) # type: ignore[deprecated] - - def test_permit_connection_error_is_still_a_permit_error() -> None: error = PermitConnectionError("boom") @@ -525,45 +516,6 @@ def test_permit_connection_error_is_still_a_permit_error() -> None: assert error.original_error is None -def test_importing_the_sdk_emits_no_deprecation_warning() -> None: - # On pydantic 1, importing permit warns on purpose that pydantic 1 support is deprecated - # (see test_fix_pydantic1_deprecation.py); the later -W option takes precedence. - result = subprocess.run( - [ - sys.executable, - "-W", - "error::DeprecationWarning", - "-W", - "ignore:Support for pydantic 1 is deprecated:DeprecationWarning", - "-c", - "import permit", - ], - capture_output=True, - text=True, - check=False, - ) - - assert result.returncode == 0, result.stderr - - -def test_permit_exception_still_warns_when_instantiated() -> None: - with pytest.warns(DeprecationWarning, match="Use PermitError instead"): - exceptions.PermitException("boom") # type: ignore[deprecated] - - -def test_permit_exception_still_warns_when_subclassed() -> None: - with pytest.warns(DeprecationWarning, match="Use PermitError instead"): - - class _Custom(exceptions.PermitException): # type: ignore[deprecated] - pass - - -def test_permit_connection_error_instantiation_does_not_warn() -> None: - with warnings.catch_warnings(): - warnings.simplefilter("error") - PermitConnectionError("boom") - - def test_check_query_context_is_optional() -> None: # bulk_check reads each check's context with .get(), so a query without one # is valid and the TypedDict must not make type checkers demand it. diff --git a/tests/type_check/consumer.py b/tests/type_check/consumer.py index d6a5aabd..6723b1a9 100644 --- a/tests/type_check/consumer.py +++ b/tests/type_check/consumer.py @@ -12,6 +12,7 @@ from typing_extensions import assert_type +import permit as permit_package from permit import ( Permit, PermitApiError, @@ -43,6 +44,9 @@ UserCreateBulkOperationResult, ) from permit.enforcement.enforcer import CheckQuery + +# Deprecated: permit 4.0 removes PermitException (see mistakes_stay_errors). +from permit.exceptions import PermitException # type: ignore[deprecated] from permit.pdp_api.models import RoleAssignment from permit.sync import Permit as SyncPermit @@ -287,3 +291,13 @@ async def mistakes_stay_errors() -> None: pass async with sync_permit: # type: ignore[misc] pass + # PermitException still catches connection errors, but importing it stays an error. + try: + await permit.check("user", "read", "document") + except PermitException as error: + assert_type(error, PermitException) + # The package re-exports it for type checkers, so reading it there is deprecated too. + _ = permit_package.PermitException # type: ignore[deprecated] + # A name the modules lack stays an error: type checkers do not see their __getattr__. + _ = permit_package.PermitExceptions # type: ignore[attr-defined] + _ = permit_package.exceptions.PermitExceptions # type: ignore[attr-defined] diff --git a/tests/type_check/mypy.ini b/tests/type_check/mypy.ini index a89f927c..12d7aed1 100644 --- a/tests/type_check/mypy.ini +++ b/tests/type_check/mypy.ini @@ -1,10 +1,12 @@ # How tests/test_typing_surface.py type-checks consumer.py: the way a user's project # sees an installed permit. There is deliberately no pydantic plugin, because users -# must not need one. strict is the strictest setting a user may run, and it includes -# warn_unused_ignores and no_implicit_reexport, which only counts names a module -# exports explicitly. +# must not need one. strict and the deprecated error code are the strictest settings a +# user may run. strict includes warn_unused_ignores and no_implicit_reexport, which only +# counts names a module exports explicitly. The deprecated code, off by default, reports +# names marked deprecated (PEP 702). [mypy] strict = True +enable_error_code = deprecated # mypy does not report errors inside an installed package to its users. permit is # checked here from the source tree, so hide its own errors the same way, including