Skip to content

Squash DRF deprecation warnings - #97

Merged
jmbowman merged 3 commits into
masterfrom
jmbowman/squash_warnings
Nov 22, 2020
Merged

Squash DRF deprecation warnings#97
jmbowman merged 3 commits into
masterfrom
jmbowman/squash_warnings

Conversation

@jmbowman

@jmbowman jmbowman commented Nov 21, 2020

Copy link
Copy Markdown
Contributor

The test suite had been reporting 3 deprecation warnings (which were preventing a Django REST Framework upgrade) and a ResourceWarning:

user_tasks/views.py:57
  /home/travis/build/edx/django-user-tasks/user_tasks/views.py:57: RemovedInDRF310Warning: `detail_route` is deprecated and will be removed in 3.10 in favor of `action`, which accepts a `detail` bool. Use `@action(detail=True)` instead.
    @detail_route(methods=['post'])

user_tasks/urls.py:10
  /home/travis/build/edx/django-user-tasks/user_tasks/urls.py:10: RemovedInDRF311Warning: The `base_name` argument is pending deprecation in favor of `basename`.
    ROUTER.register(r'artifacts', ArtifactViewSet, base_name='usertaskartifact')

user_tasks/urls.py:11
  /home/travis/build/edx/django-user-tasks/user_tasks/urls.py:11: RemovedInDRF311Warning: The `base_name` argument is pending deprecation in favor of `basename`.
    ROUTER.register(r'tasks', StatusViewSet, base_name='usertaskstatus')

/opt/python/3.8.0/lib/python3.8/tempfile.py:816: ResourceWarning: Implicitly cleaning up <TemporaryDirectory '/tmp/tmpxp8ex7s_'>
  _warnings.warn(warn_message, ResourceWarning)

The DRF deprecation warnings were also appearing in the edx-platform test suite, since it uses this package. This should pave the way to upgrading DRF in edx-platform to at least 3.11, since these were the last deprecation warnings reporting breakage that would occur with that version.

21:39:22  ../../edx-venv-3.8/edx-venv/lib/python3.8/site-packages/user_tasks/views.py:57
21:39:22  ../../edx-venv-3.8/edx-venv/lib/python3.8/site-packages/user_tasks/views.py:57
21:39:22  ../../edx-venv-3.8/edx-venv/lib/python3.8/site-packages/user_tasks/views.py:57
21:39:22    /home/jenkins/edx-venv-3.8/edx-venv/lib/python3.8/site-packages/user_tasks/views.py:57: RemovedInDRF310Warning: `detail_route` is deprecated and will be removed in 3.10 in favor of `action`, which accepts a `detail` bool. Use `@action(detail=True)` instead.
21:39:22      @detail_route(methods=['post'])
21:39:22  
21:39:22  ../../edx-venv-3.8/edx-venv/lib/python3.8/site-packages/user_tasks/urls.py:10
21:39:22  ../../edx-venv-3.8/edx-venv/lib/python3.8/site-packages/user_tasks/urls.py:10
21:39:22  ../../edx-venv-3.8/edx-venv/lib/python3.8/site-packages/user_tasks/urls.py:10
21:39:22    /home/jenkins/edx-venv-3.8/edx-venv/lib/python3.8/site-packages/user_tasks/urls.py:10: RemovedInDRF311Warning: The `base_name` argument is pending deprecation in favor of `basename`.
21:39:22      ROUTER.register(r'artifacts', ArtifactViewSet, base_name='usertaskartifact')
21:39:22  
21:39:22  ../../edx-venv-3.8/edx-venv/lib/python3.8/site-packages/user_tasks/urls.py:11
21:39:22  ../../edx-venv-3.8/edx-venv/lib/python3.8/site-packages/user_tasks/urls.py:11
21:39:22  ../../edx-venv-3.8/edx-venv/lib/python3.8/site-packages/user_tasks/urls.py:11
21:39:22    /home/jenkins/edx-venv-3.8/edx-venv/lib/python3.8/site-packages/user_tasks/urls.py:11: RemovedInDRF311Warning: The `base_name` argument is pending deprecation in favor of `basename`.
21:39:22      ROUTER.register(r'tasks', StatusViewSet, base_name='usertaskstatus')

Implementation notes:

  • The changes to DRF usage are exact equivalents to the previous code, behavior shouldn't change.
  • Added testing with each major DRF version from 3.9.x (used in edx-platform) to the latest 3.12.x
  • Using a pytest session fixture to clean up the temporary directories when the test session ends. I had to upper case the variable names for them to be importable via from django.conf import settings, since test_settings.py itself isn't in an importable location.

@codecov

codecov Bot commented Nov 21, 2020

Copy link
Copy Markdown

Codecov Report

Merging #97 (4f426b0) into master (4e0046e) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##            master       #97   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            8         8           
  Lines          359       359           
  Branches        47        47           
=========================================
  Hits           359       359           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4e0046e...4f426b0. Read the comment docs.

@jmbowman
jmbowman requested a review from a team November 21, 2020 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants