Skip to content

Conversation

hmpf
Copy link
Contributor

@hmpf hmpf commented Oct 22, 2021

Lots of things that were introduced in 2.2 replaces things that are removed in 4.0. When running on 3.2, this leads to various types of warnings. Depends on #2317.

Warnings triggered by Python itself, for pythons currently not supported by NAV like 3.9 or 3.10, will also be collected here.

@hmpf hmpf mentioned this pull request Oct 22, 2021
@github-actions
Copy link

github-actions bot commented Oct 22, 2021

Test results

       12 files         12 suites   16m 29s ⏱️
  2 805 tests   2 709 ✔️   96 💤 0
11 220 runs  10 836 ✔️ 384 💤 0

Results for commit 104bc99.

♻️ This comment has been updated with latest results.

@hmpf
Copy link
Contributor Author

hmpf commented Oct 22, 2021

The following warning is raised by dependencies:

DeprecationWarning: Using or importing the ABCs from 'collections' instead of from 'collections.abc' is deprecated since Python 3.3,and in 3.9 it will stop working

The dependencies in question are: IPy, napalm, networkx.

@hmpf
Copy link
Contributor Author

hmpf commented Oct 22, 2021

The following warning should probably not be handled before we are running on Django 3.2:

(fields.W904)
django.contrib.postgres.fields.JSONField is deprecated. Support for it (except in
historical migrations) will be removed in Django 4.0.

The replacement, django.db.models.JSONField, isn't available on Django 2.2.

@hmpf hmpf marked this pull request as ready for review October 22, 2021 12:58
@hmpf hmpf requested a review from lunkwill42 October 22, 2021 12:58
@hmpf hmpf force-pushed the get-rid-of-warnings branch 2 times, most recently from 9d3a513 to a5308d9 Compare November 29, 2021 08:51
hmpf added 8 commits December 3, 2021 07:37
Gets rid of the following warnings:

RemovedInDjango40Warning:
The {% ifequal %} template tag is deprecated in favor of {% if %}.

RemovedInDjango40Warning:
The {% ifnotequal %} template tag is deprecated in favor of {% if %}.
Gets rid of warning:

RemovedInDjango40Warning:
django.conf.urls.url() is deprecated in favor of django.urls.re_path().
Gets rid of warning:

models.TimePeriod: (models.W042) Auto-created primary key used when not
defining a primary key type, by default 'django.db.models.AutoField'.
        HINT: Configure the DEFAULT_AUTO_FIELD setting or the
        NavModelsConfig.default_auto_field attribute to point to
        a subclass of AutoField, e.g. 'django.db.models.BigAutoField'.
Get rid of warning:

RemovedInDjango40Warning:
  The providing_args argument is deprecated. As it is purely
  documentational, it has no replacement. If you rely on this argument as
  documentation, you can move the text to a code comment or docstring.
Gets rid of warning:

RemovedInDjango41Warning:
  'blapp' defines default_app_config = 'blapp.apps.BlappConfig'. Django
  now detects this configuration automatically. You can remove
  default_app_config.
Gets rid of warning:

RemovedInDjango40Warning:
  Passing None for the middleware get_response argument is deprecated.
Gets rid of warning:

RemovedInDjango40Warning:
  django.utils.translation.ugettext() is deprecated in favor of
  django.utils.translation.gettext().
Gets rid of warning:

(fields.W903)
  NullBooleanField is deprecated. Support for it (except in historical
  migrations) will be removed in Django 4.0.
@hmpf hmpf force-pushed the get-rid-of-warnings branch from a5308d9 to 9043bf9 Compare December 3, 2021 06:39
@codecov
Copy link

codecov bot commented Dec 3, 2021

Codecov Report

Merging #2318 (104bc99) into master (2631e34) will decrease coverage by 0.00%.
The diff coverage is 97.91%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2318      +/-   ##
==========================================
- Coverage   58.02%   58.02%   -0.01%     
==========================================
  Files         554      552       -2     
  Lines       40211    40207       -4     
==========================================
- Hits        23334    23330       -4     
  Misses      16877    16877              
Impacted Files Coverage Δ
python/nav/auditlog/__init__.py 100.00% <ø> (ø)
python/nav/models/__init__.py 100.00% <ø> (ø)
python/nav/web/__init__.py 82.35% <ø> (-0.51%) ⬇️
python/nav/django/validators.py 92.30% <50.00%> (ø)
python/nav/auditlog/urls.py 100.00% <100.00%> (ø)
python/nav/django/settings.py 97.56% <100.00%> (+0.03%) ⬆️
python/nav/django/urls.py 69.23% <100.00%> (ø)
python/nav/ipdevpoll/signals.py 100.00% <100.00%> (ø)
python/nav/models/manage.py 75.27% <100.00%> (ø)
python/nav/web/ajax/urls.py 100.00% <100.00%> (ø)
... and 39 more

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 2631e34...104bc99. Read the comment docs.

Copy link
Member

@lunkwill42 lunkwill42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, I'd say this looks peachy. The burning question is whether this completely moves NAV to require Django >= 3.2...

Copy link
Member

@lunkwill42 lunkwill42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on feedback, I am correct in assuming this PR solidly takes us into Django>=3.2-land, which means a few more things are needed:

  • /requirements.txt still requires Django 2.2. This needs an update.
  • /NOTES.rst needs a note about all the changed dependencies.
  • /doc/howto/generic-install-from-source.txt still includes a requirements list for Django 1.11, this needs an update.

@hmpf hmpf requested a review from lunkwill42 December 3, 2021 11:28
@hmpf hmpf changed the title Get rid of warnings when running on Django 3.2 Switch to Django 3.2 and get rid of warnings Dec 3, 2021
Copy link
Member

@lunkwill42 lunkwill42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And just in time for the weekend! :)

@lunkwill42 lunkwill42 merged commit 0c8f4cf into Uninett:master Dec 3, 2021
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.

2 participants