Skip to content
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

Type checks started failing on our codebase with v4.4 #514

Closed
5 tasks done
lorinkoz opened this issue Aug 18, 2023 · 13 comments · Fixed by #515 or #521
Closed
5 tasks done

Type checks started failing on our codebase with v4.4 #514

lorinkoz opened this issue Aug 18, 2023 · 13 comments · Fixed by #515 or #521
Labels
bug This points to a verified bug in the code

Comments

@lorinkoz
Copy link
Contributor

Checklist

  • I have looked into the Readme and Examples, and have not found a suitable solution or answer.
  • I have looked into the API documentation and have not found a suitable solution or answer.
  • I have searched the issues and have not found a suitable solution or answer.
  • I have searched the Auth0 Community forums and have not found a suitable solution or answer.
  • I agree to the terms within the Auth0 Code of Conduct.

Description

No complaints on v4.3, automatic upgrade PR on v4.4 fails like this:

Screenshot 2023-08-18 at 08 53 26

Example of the code:

import auth0.management as _management

token = get_token_from_somewhere()
auth0 = _management.Auth0(API_DOMAIN, token)

users = auth0.users_by_email.search_users_by_email(...)

Reproduction

[Nothing useful to add here]

Additional context

No response

auth0-python version

4.4

Python version

3.11

@lorinkoz lorinkoz added the bug This points to a verified bug in the code label Aug 18, 2023
@adamjmcgrath
Copy link
Contributor

Thanks @lorinkoz - will take a look

@dazzag24
Copy link

I'm also seeing this issue after just updating to 4.4

@lorinkoz
Copy link
Contributor Author

Thanks for fixing so quickly! Do you plan to do a patch release for this change alone or do you normally wait for a few more fixes before cutting a release?

@adamjmcgrath
Copy link
Contributor

Np @lorinkoz - will ship a patch today

@adamjmcgrath
Copy link
Contributor

@lorinkoz - apologies - it might take a little longer while I fix a releasing issue

Will update this thread when the patch is up

@adamjmcgrath
Copy link
Contributor

@lorinkoz 4.4.1 is now available - lmk if you run into any more types issues in the management client

@lorinkoz
Copy link
Contributor Author

Good day. FYI, the problem remains. I just checked the PR and it seems to be adjusting the mypy config for this repo, but this doesn't have any effect on projects using the package.

@adamjmcgrath
Copy link
Contributor

Ah, ok - thanks for the heads up @lorinkoz

@Viicos - do you know a fix for this?

@adamjmcgrath adamjmcgrath reopened this Aug 30, 2023
@Viicos
Copy link
Contributor

Viicos commented Aug 30, 2023

Well this looks similar to #483 but for the AsyncAuth0 class. Can be fixed by either doing the same thing as:

def __init__(
self, domain: str, token: str, rest_options: RestClientOptions | None = None
):
self.actions = Actions(domain, token, rest_options=rest_options)
self.attack_protection = AttackProtection(
domain, token, rest_options=rest_options
)
self.blacklists = Blacklists(domain, token, rest_options=rest_options)
self.branding = Branding(domain, token, rest_options=rest_options)
self.client_credentials = ClientCredentials(
domain, token, rest_options=rest_options
)
self.client_grants = ClientGrants(domain, token, rest_options=rest_options)
self.clients = Clients(domain, token, rest_options=rest_options)
self.connections = Connections(domain, token, rest_options=rest_options)
self.custom_domains = CustomDomains(domain, token, rest_options=rest_options)
self.device_credentials = DeviceCredentials(
domain, token, rest_options=rest_options
)
self.email_templates = EmailTemplates(domain, token, rest_options=rest_options)
self.emails = Emails(domain, token, rest_options=rest_options)
self.grants = Grants(domain, token, rest_options=rest_options)
self.guardian = Guardian(domain, token, rest_options=rest_options)
self.hooks = Hooks(domain, token, rest_options=rest_options)
self.jobs = Jobs(domain, token, rest_options=rest_options)
self.log_streams = LogStreams(domain, token, rest_options=rest_options)
self.logs = Logs(domain, token, rest_options=rest_options)
self.organizations = Organizations(domain, token, rest_options=rest_options)
self.prompts = Prompts(domain, token, rest_options=rest_options)
self.resource_servers = ResourceServers(
domain, token, rest_options=rest_options
)
self.roles = Roles(domain, token, rest_options=rest_options)
self.rules_configs = RulesConfigs(domain, token, rest_options=rest_options)
self.rules = Rules(domain, token, rest_options=rest_options)
self.stats = Stats(domain, token, rest_options=rest_options)
self.tenants = Tenants(domain, token, rest_options=rest_options)
self.tickets = Tickets(domain, token, rest_options=rest_options)
self.user_blocks = UserBlocks(domain, token, rest_options=rest_options)
self.users_by_email = UsersByEmail(domain, token, rest_options=rest_options)
self.users = Users(domain, token, rest_options=rest_options)

Or by doing it in a TYPE_CHECKING block

@adamjmcgrath
Copy link
Contributor

@Viicos We add async variants to every method in the management client at runtime - eg users.get_async(id) would all of these not also need to be defined? can we just opt out of type checking for AsyncAuth0?

@Viicos
Copy link
Contributor

Viicos commented Aug 30, 2023

Looking at how it is done currently:

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,
)

And the fact that asyncify returns dynamic stuff as well, it is indeed hard to type check this correctly.

The fix you did on #515 only affects the current project, as the mypy.ini file is only used when working on auth0-python. Instead, projects depending on auth0-python need to disable this attr-defined error code on their mypy configuration if they want to ignore the error (this means that #515 can be reverted)

@adamjmcgrath
Copy link
Contributor

Instead, projects depending on auth0-python need to disable this attr-defined error code on their mypy configuration

I don't think It's reasonable to expect users of this package to have to do this, if there isn't a better solution I'm going to have to remove the py.typed file and revert publishing types until a better solution is available.

github-merge-queue bot pushed a commit that referenced this issue Aug 31, 2023
### Changes

This package started publishing types in 4.4, but the way we add async
classes and methods makes type checking unfeasible.

Revert publishing types externally until a better solution is available
(this will probably be by generating the SDK).

### References

fixes #514
See
https://mypy.readthedocs.io/en/stable/installed_packages.html#creating-pep-561-compatible-packages
@lorinkoz
Copy link
Contributor Author

lorinkoz commented Sep 7, 2023

Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This points to a verified bug in the code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants