test type annotations with mypy #668
Conversation
| else: | ||
| if connection is None: | ||
| connection = queue.connection |
There was a problem hiding this comment.
I think this was an oversight in the previous code?
|
|
||
| RQ = getattr(settings, 'RQ', {}) | ||
| default_result_ttl = RQ.get('DEFAULT_RESULT_TTL') | ||
| if default_result_ttl is not None: | ||
| kwargs.setdefault('result_ttl', default_result_ttl) | ||
|
|
||
| decorator = _rq_job(queue, connection=connection, *args, **kwargs) | ||
| kwargs['connection'] = connection |
There was a problem hiding this comment.
This is just for mypy (python/mypy#6799) and rather than adding a type ignore I figured I'd update the kwargs, since it would never contain the connection based on the function's definition
| @@ -22,9 +22,7 @@ def handle(self, *args, **options): | |||
| Queues the function given with the first argument with the | |||
| parameters given with the rest of the argument list. | |||
| """ | |||
| verbosity = int(options.get('verbosity', 1)) | |||
There was a problem hiding this comment.
These will all exist, and using indexed access will tell mypy they are not None
| - name: Set up Python 3.8 | ||
| uses: actions/[email protected] | ||
| with: | ||
| python-version: "3.8" |
There was a problem hiding this comment.
Picking an old supported Python version since it doesn't it's tested otherwise, and this will make sure the type annotations don't break the older Pythons
django_rq/py.typed
Outdated
There was a problem hiding this comment.
Export type annotations
| @@ -722,7 +718,7 @@ def test_commandline_verbosity_affects_logging_level(self, setup_loghandlers_moc | |||
| setup_loghandlers_mock.assert_called_once_with(expected_level[verbosity]) | |||
|
|
|||
| @override_settings(RQ={'SCHEDULER_CLASS': 'django_rq.tests.fixtures.DummyScheduler'}) | |||
| def test_scheduler_default_timeout(self): | |||
| def test_scheduler_default(self): | |||
There was a problem hiding this comment.
This test was overridden by the one that comes after it (thanks mypy!)
| """Fetch jobs in bulk from Redis. | ||
| 1. If job data is not present in Redis, discard the result | ||
| 2. If `registry` argument is supplied, delete empty jobs from registry | ||
| """ | ||
| jobs = Job.fetch_many(job_ids, connection=queue.connection, serializer=queue.serializer) | ||
| jobs = cast( | ||
| List[Optional[Job]], Job.fetch_many(job_ids, connection=queue.connection, serializer=queue.serializer) |
There was a problem hiding this comment.
this is a bug in fetch_many and mypy will complain that job is None can't happen otherwise
| @@ -403,7 +404,12 @@ def clear_queue(request, queue_index): | |||
| queue.empty() | |||
| messages.info(request, 'You have successfully cleared the queue %s' % queue.name) | |||
| except ResponseError as e: | |||
| if 'EVALSHA' in e.message: | |||
| try: | |||
| suppress = 'EVALSHA' in e.message # type: ignore[attr-defined] | |||
There was a problem hiding this comment.
I can't find this ever existing, but it seems to have worked for the PR author?
| [mypy] | ||
| allow_redefinition = true | ||
| check_untyped_defs = true |
There was a problem hiding this comment.
treat untyped code as Any to make sure everything is checked even if it ends up not being very useful
| warn_redundant_casts = true | ||
| warn_unused_ignores = true | ||
| warn_unreachable = true |
There was a problem hiding this comment.
warn in case mypy thinks this case can never happen (in case it actually can and the type checking is not checking a path that should be checked)
terencehonles
force-pushed
the
mypy-type-checking
branch
from
62918d2
to
0a02065
Compare
I was going to contribute my typing for the
@jobdecorator that was missing from #656, but I then realized there was no way to check if the annotations were correct, so I addedmypyto the testing matrix