-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[auth0-python] Add async functions to AsyncAuth0 #13799
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,73 @@ | ||
from _typeshed import Incomplete | ||
from types import TracebackType | ||
from typing_extensions import Self | ||
|
||
from auth0.rest import RestClientOptions as RestClientOptions | ||
|
||
from ..asyncify import asyncify as asyncify | ||
from .actions import Actions as Actions | ||
from .attack_protection import AttackProtection as AttackProtection | ||
from .auth0 import Auth0 as Auth0 | ||
from .blacklists import Blacklists as Blacklists | ||
from .branding import Branding as Branding | ||
from .client_credentials import ClientCredentials as ClientCredentials | ||
from .client_grants import ClientGrants as ClientGrants | ||
from .clients import Clients as Clients | ||
from .connections import Connections as Connections | ||
from .custom_domains import CustomDomains as CustomDomains | ||
from .device_credentials import DeviceCredentials as DeviceCredentials | ||
from .email_templates import EmailTemplates as EmailTemplates | ||
from .emails import Emails as Emails | ||
from .grants import Grants as Grants | ||
from .guardian import Guardian as Guardian | ||
from .hooks import Hooks as Hooks | ||
from .jobs import Jobs as Jobs | ||
from .log_streams import LogStreams as LogStreams | ||
from .logs import Logs as Logs | ||
from .organizations import Organizations as Organizations | ||
from .prompts import Prompts as Prompts | ||
from .resource_servers import ResourceServers as ResourceServers | ||
from .roles import Roles as Roles | ||
from .rules import Rules as Rules | ||
from .rules_configs import RulesConfigs as RulesConfigs | ||
from .stats import Stats as Stats | ||
from .tenants import Tenants as Tenants | ||
from .tickets import Tickets as Tickets | ||
from .user_blocks import UserBlocks as UserBlocks | ||
from .users import Users as Users | ||
from .users_by_email import UsersByEmail as UsersByEmail | ||
|
||
class AsyncAuth0: | ||
actions: Incomplete | ||
attack_protection: Incomplete | ||
blacklists: Incomplete | ||
branding: Incomplete | ||
client_credentials: Incomplete | ||
client_grants: Incomplete | ||
clients: Incomplete | ||
connections: Incomplete | ||
custom_domains: Incomplete | ||
device_credentials: Incomplete | ||
email_templates: Incomplete | ||
emails: Incomplete | ||
grants: Incomplete | ||
guardian: Incomplete | ||
hooks: Incomplete | ||
jobs: Incomplete | ||
log_streams: Incomplete | ||
logs: Incomplete | ||
organizations: Incomplete | ||
prompts: Incomplete | ||
resource_servers: Incomplete | ||
roles: Incomplete | ||
rules_configs: Incomplete | ||
rules: Incomplete | ||
stats: Incomplete | ||
tenants: Incomplete | ||
tickets: Incomplete | ||
user_blocks: Incomplete | ||
users_by_email: Incomplete | ||
users: Incomplete | ||
Comment on lines
+41
to
+70
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note that You may want to request this dynamic typing upstream as well. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The reason I added these type hints is because the _async functions aren't typed because they don't exist except at run time. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All these attributes exist at runtime because they're dynamically generated by class AsyncAuth0:
def __init__(
self, domain: str, token: str, rest_options: RestClientOptions | None = None
) -> None:
self._services = []
for name, attr in vars(Auth0(domain, token, rest_options=rest_options)).items():
cls = asyncify(attr.__class__)
service = cls(domain=domain, token=token, rest_options=rest_options)
self._services.append(service)
setattr(
self,
name,
service,
) That's why static checkers and stubtest couldn't find them.
From what I just read in auth0/auth0-python#521 . These attributes are the exact reason why In case it's not clear, this comment isn't asking for any modification. I'm just taking note of the state of static typing upstream of this stub. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, exactly. I created a PR to add stubs to Which is why I added these stubs to type shed. I'm concerned that if you add the py.typed, I'll be back at square one - with no types for _async. |
||
def __init__(self, domain: str, token: str, rest_options: RestClientOptions | None = None) -> None: ... | ||
def set_session(self, session) -> None: ... | ||
async def __aenter__(self) -> Self: ... | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just noticed that these are all re-exported from this file. I don't think we should do that and just use normal import for the items we need in this file (I.e. just
from .tenants import Tenants
without theas Tenants
part.) This applies to all imports, not just the new ones.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I found with this is that my IDE considers these unused if I remove the
as
import - should I ignore that?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In that case you can just remove the unused imports.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, by importing these they become attributes of the class - I think this is the correct approach and you should approve as is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@srittau Bump, thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure about these?
You can't affect what attributes are seen as part of a class simply by importing in a stub. I don't understand what your expectations are here.