Page MenuHomeSoftware Heritage

api/throttling: Add mypy type annotations
ClosedPublic

Authored by anlambert on Thu, Mar 19, 5:56 PM.

Details

Summary

Add mypy type annotations in swh.web.api.throttling.

Diff Detail

Repository
rDWAPPS Web applications
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

anlambert created this revision.Thu, Mar 19, 5:56 PM
vlorentz requested changes to this revision.Fri, Mar 20, 11:35 AM
vlorentz added a subscriber: vlorentz.
vlorentz added inline comments.
swh/web/api/throttling.py
74

why remove is not None?

88–89

Could you add a test for request.method being None?

151

What about this?

T = TypeVar('T', Callable)
def decorator(func: T) -> T:

And this might allow you to completely specify the type of throttle_scope.

This revision now requires changes to proceed.Fri, Mar 20, 11:35 AM
anlambert added inline comments.Fri, Mar 20, 2:10 PM
swh/web/api/throttling.py
74

I got mistaken by mypy, will revert.

88–89

This should never be the case (https://stackoverflow.com/questions/7836834/how-can-request-method-none-in-django) so no test needed.

I was forced to do that as django-stubs types method attribute as Optional[str] because it is initialized to None in HttpRequest constructor. So I got the following
mypy error:

swh/web/api/throttling.py:83: error: Item "None" of "Optional[str]" has no attribute "lower"

I will go for a cast instead:

method = cast(str, request.method)
request_scope = f'{default_scope}_{method.lower()}'
151

Thanks for the tip, got it working with:

from rest_framework.views import APIView
View = TypeVar('View', bound=APIView)

def decorator(func: View) -> View:
vlorentz added inline comments.Fri, Mar 20, 2:21 PM
swh/web/api/throttling.py
88–89

I'd rather use an assertion

anlambert added inline comments.Fri, Mar 20, 2:21 PM
swh/web/api/throttling.py
151

mypy is okay but unfortunately webapp execution got broken due to recursive import:

Traceback (most recent call last):
  File "swh/web/manage.py", line 46, in <module>
    execute_from_command_line(sys.argv)
  File "/home/antoine/.virtualenvs/swh/lib/python3.7/site-packages/django/core/management/__init__.py", line 381, in execute_from_command_line
    utility.execute()
  File "/home/antoine/.virtualenvs/swh/lib/python3.7/site-packages/django/core/management/__init__.py", line 375, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/home/antoine/.virtualenvs/swh/lib/python3.7/site-packages/django/core/management/base.py", line 323, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/home/antoine/.virtualenvs/swh/lib/python3.7/site-packages/django/core/management/base.py", line 361, in execute
    self.check()
  File "/home/antoine/.virtualenvs/swh/lib/python3.7/site-packages/django/core/management/base.py", line 390, in check
    include_deployment_checks=include_deployment_checks,
  File "/home/antoine/.virtualenvs/swh/lib/python3.7/site-packages/django/core/management/commands/migrate.py", line 65, in _run_checks
    issues.extend(super()._run_checks(**kwargs))
  File "/home/antoine/.virtualenvs/swh/lib/python3.7/site-packages/django/core/management/base.py", line 377, in _run_checks
    return checks.run_checks(**kwargs)
  File "/home/antoine/.virtualenvs/swh/lib/python3.7/site-packages/django/core/checks/registry.py", line 72, in run_checks
    new_errors = check(app_configs=app_configs)
  File "/home/antoine/.virtualenvs/swh/lib/python3.7/site-packages/django/core/checks/urls.py", line 13, in check_url_config
    return check_resolver(resolver)
  File "/home/antoine/.virtualenvs/swh/lib/python3.7/site-packages/django/core/checks/urls.py", line 23, in check_resolver
    return check_method()
  File "/home/antoine/.virtualenvs/swh/lib/python3.7/site-packages/django/urls/resolvers.py", line 399, in check
    for pattern in self.url_patterns:
  File "/home/antoine/.virtualenvs/swh/lib/python3.7/site-packages/django/utils/functional.py", line 80, in __get__
    res = instance.__dict__[self.name] = self.func(instance)
  File "/home/antoine/.virtualenvs/swh/lib/python3.7/site-packages/django/urls/resolvers.py", line 584, in url_patterns
    patterns = getattr(self.urlconf_module, "urlpatterns", self.urlconf_module)
  File "/home/antoine/.virtualenvs/swh/lib/python3.7/site-packages/django/utils/functional.py", line 80, in __get__
    res = instance.__dict__[self.name] = self.func(instance)
  File "/home/antoine/.virtualenvs/swh/lib/python3.7/site-packages/django/urls/resolvers.py", line 577, in urlconf_module
    return import_module(self.urlconf_name)
  File "/home/antoine/.virtualenvs/swh/lib/python3.7/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1006, in _gcd_import
  File "<frozen importlib._bootstrap>", line 983, in _find_and_load
  File "<frozen importlib._bootstrap>", line 967, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 677, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 728, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "/home/antoine/swh/swh-environment/swh-web/swh/web/urls.py", line 38, in <module>
    url(r'^api/', include('swh.web.api.urls')),
  File "/home/antoine/.virtualenvs/swh/lib/python3.7/site-packages/django/urls/conf.py", line 34, in include
    urlconf_module = import_module(urlconf_module)
  File "/home/antoine/.virtualenvs/swh/lib/python3.7/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1006, in _gcd_import
  File "<frozen importlib._bootstrap>", line 983, in _find_and_load
  File "<frozen importlib._bootstrap>", line 967, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 677, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 728, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "/home/antoine/swh/swh-environment/swh-web/swh/web/api/urls.py", line 6, in <module>
    import swh.web.api.views.content # noqa
  File "/home/antoine/swh/swh-environment/swh-web/swh/web/api/views/content.py", line 13, in <module>
    from swh.web.api.apidoc import api_doc, format_docstring
  File "/home/antoine/swh/swh-environment/swh-web/swh/web/api/apidoc.py", line 18, in <module>
    from rest_framework.decorators import api_view
  File "/home/antoine/.virtualenvs/swh/lib/python3.7/site-packages/rest_framework/decorators.py", line 13, in <module>
    from rest_framework.views import APIView
  File "/home/antoine/.virtualenvs/swh/lib/python3.7/site-packages/rest_framework/views.py", line 104, in <module>
    class APIView(View):
  File "/home/antoine/.virtualenvs/swh/lib/python3.7/site-packages/rest_framework/views.py", line 110, in APIView
    throttle_classes = api_settings.DEFAULT_THROTTLE_CLASSES
  File "/home/antoine/.virtualenvs/swh/lib/python3.7/site-packages/rest_framework/settings.py", line 220, in __getattr__
    val = perform_import(val, attr)
  File "/home/antoine/.virtualenvs/swh/lib/python3.7/site-packages/rest_framework/settings.py", line 168, in perform_import
    return [import_from_string(item, setting_name) for item in val]
  File "/home/antoine/.virtualenvs/swh/lib/python3.7/site-packages/rest_framework/settings.py", line 168, in <listcomp>
    return [import_from_string(item, setting_name) for item in val]
  File "/home/antoine/.virtualenvs/swh/lib/python3.7/site-packages/rest_framework/settings.py", line 180, in import_from_string
    raise ImportError(msg)
ImportError: Could not import 'swh.web.api.throttling.SwhWebRateThrottle' for API setting 'DEFAULT_THROTTLE_CLASSES'. ImportError: cannot import name 'APIView' from 'rest_framework.views' (/home/antoine/.virtualenvs/swh/lib/python3.7/site-packages/rest_framework/views.py)

I need to find another solution.

anlambert added inline comments.Fri, Mar 20, 2:22 PM
swh/web/api/throttling.py
88–89

Oh right !

vlorentz added inline comments.Fri, Mar 20, 2:27 PM
swh/web/api/throttling.py
151

What about this? View = TypeVar('View', bound='rest_framework.views.APIView')

anlambert added inline comments.Fri, Mar 20, 2:40 PM
swh/web/api/throttling.py
151

It worked, thanks !

anlambert updated this revision to Diff 10180.Fri, Mar 20, 2:41 PM

Update: address @vlorentz comments

vlorentz accepted this revision.Fri, Mar 20, 2:44 PM
This revision is now accepted and ready to land.Fri, Mar 20, 2:44 PM
This revision was landed with ongoing or failed builds.Fri, Mar 20, 2:47 PM
This revision was automatically updated to reflect the committed changes.