This PR upgrades to Django 3.2 and also upgrades most dependencies within the Django and analysis containers.
Notes
Analysis Container Update
This PR is based on work by @jacobscarfmerrell and @KlaasH on #876; in the interest of citing my sources, I want to make it clear that due to a particularly grueling rebase process, a lot of that work is now represented by commits d8f59d0 and 9cc8be2.
Invalid Literal for int() Error
This PR addresses a failure while running analysis, where the the analysis process would fail right at the end while attempting to fetch the newly updated AnalysisJob object to add in the residential speed limit. Similarly, if you ran analysis on develop and then tried to view that analysis, either from the frontend or via AnalysisJob.objects.get(), you would see an invalid literal for int() of base 10 error. After quite a long time trying to follow the string back, it turns out that while the SQL command was generating the CSV with all original_score fields as having 4 decimal points, AnalysisJobManager was expecting a PositiveIntegerField while annotating each object with its population. To address this issue with new projects, this PR adds a line to clean_metric_dict() within load_overall_scores that first converts population_total to a float and then again into an integer. For past projects, this PR casts population to an integer during annotation.
As to where this error came from, I couldn’t exactly pin down which update to which package would have created this behavior. I initially thought it was the “Narrower exception catching in serializer fields, to ensure that any errors in broken get_queryset() methods are not masked” 3.12.2 update to Django Rest Framework; however, the error was ultimately not being thrown by the serializer. Similarly, Django has made upgrades to its annotations and PositiveIntegerField type, but none that would obviously result in this issue.
Errors Not Addressed in this PR
There were a few errors that I saw while working on this branch that did not appear to be a product of the upgrade and were therefore outside the scope of this PR, but may be worth investigating as separate issues:
On set up
Just prior to starting the Ansible playbook, two warnings appear:
/usr/local/lib/python3.8/dist-packages/requests/__init__.py:89: RequestsDependencyWarning: urllib3 (1.26.9)
or chardet (3.0.4) doesn't match a supported version! warnings.warn("urllib3 ({}) or chardet ({}) doesn't
match a supported "
This warning does not disappear even when the most recent version of these packages are specified in requirements.txt, which makes me think it’s either not coming from the Django or analysis containers or is being generated as a result of our outdated version of django-localflavor, which also lists these packages as dependencies.
[WARNING]: Skipping plugin (/usr/local/lib/python3.8/dist-packages/ansible/plugins/filter/core.py) as it
seems to be invalid: cannotimport name 'environmentfilter' from 'jinja2.filters'(/usr/local/lib/python3.8/
dist-packages/jinja2/filters.py)
This seems to be related to Ansible and is not fixed with an update to the Django or analysis containers.
While running Ansible playbook during ./scripts/setup, if you are running it over top of an existing VM (not creating it from scratch), an error appears during the 'Extract and install Terraform' task:
[DEPRECATION WARNING]: Using tests as filters is deprecated. Instead of using
result|changed` use `result is changed`. This feature will be removed in version 2.9.
Finally, there were a lot of deprecation warnings generated while building the angularjs container.
Running tests or server
Running tests or the server with the -Wall flag inside the Django container also results in a few errors I did not address:
A lot of django.utils.translation.ugettext_lazy() is deprecated in favor of django.utils.translation.gettext_lazy() warnings; these are a result of us keeping django-localflavor at version 2.2.
/usr/src/pfb_network_connectivity/settings.py:308: ResourceWarning: unclosed <ssl.SSLSocket fd=3,
family=AddressFamily.AF_INET, type=SocketKind.SOCK_STREAM, proto=6, laddr=('172.18.0.3',
52626), raddr=('52.73.139.254', 443)>
revision = get_latest_job_definition(PFB_AWS_BATCH_ANALYSIS_JOB_DEFINITION_NAME)['revision']
ResourceWarning: Enable tracemalloc to get the object allocation traceback
This goes back to an error I’ve seen before within boto3, which this thread seems to describe as just noise. I did double check that all <file>.opens are either preceded by with or followed by <file>.close() and then left it as is.
/usr/local/lib/python3.10/site-packages/past/builtins/misc.py:45: DeprecationWarning: the imp module is
deprecated in favour of importlib and slated for removal in Python 3.12; see the module's documentation
for alternative uses
from imp import reload
Verify that previously made projects are displaying as they should
Run a new analysis and verify it completes and displays on the analysis jobs page as expected
Back in the browser, verify that you can see the place you just analyzed on http://localhost:9301/#/places//// and that the high and low stress routes appear as expected
Verify that you can compare places
In the VM, run docker-compose run --rm --entrypoint bash django and then run python3 -Wall manage.py test --noinput
Verify that the only deprecation warnings you see are listed above and that the tests complete without errors
Still in the docker container, run python3 -Wall manage.py runserver and verify that:
the only deprecation warnings you see are listed above
Overview
This PR upgrades to Django 3.2 and also upgrades most dependencies within the Django and analysis containers.
Notes
Analysis Container Update
This PR is based on work by @jacobscarfmerrell and @KlaasH on #876; in the interest of citing my sources, I want to make it clear that due to a particularly grueling rebase process, a lot of that work is now represented by commits d8f59d0 and 9cc8be2.
Invalid Literal for int() Error
This PR addresses a failure while running analysis, where the the analysis process would fail right at the end while attempting to fetch the newly updated
AnalysisJobobject to add in the residential speed limit. Similarly, if you ran analysis ondevelopand then tried to view that analysis, either from the frontend or viaAnalysisJob.objects.get(), you would see aninvalid literal for int() of base 10error. After quite a long time trying to follow the string back, it turns out that while the SQL command was generating the CSV with alloriginal_scorefields as having 4 decimal points,AnalysisJobManagerwas expecting aPositiveIntegerFieldwhile annotating each object with its population. To address this issue with new projects, this PR adds a line toclean_metric_dict()withinload_overall_scoresthat first convertspopulation_totalto a float and then again into an integer. For past projects, this PR casts population to an integer during annotation.As to where this error came from, I couldn’t exactly pin down which update to which package would have created this behavior. I initially thought it was the “Narrower exception catching in serializer fields, to ensure that any errors in broken
get_queryset()methods are not masked” 3.12.2 update to Django Rest Framework; however, the error was ultimately not being thrown by the serializer. Similarly, Django has made upgrades to its annotations andPositiveIntegerFieldtype, but none that would obviously result in this issue.Errors Not Addressed in this PR
There were a few errors that I saw while working on this branch that did not appear to be a product of the upgrade and were therefore outside the scope of this PR, but may be worth investigating as separate issues:
On set up Just prior to starting the Ansible playbook, two warnings appear:
This warning does not disappear even when the most recent version of these packages are specified in
requirements.txt, which makes me think it’s either not coming from the Django or analysis containers or is being generated as a result of our outdated version ofdjango-localflavor, which also lists these packages as dependencies.This seems to be related to Ansible and is not fixed with an update to the Django or analysis containers.
While running Ansible playbook during
./scripts/setup, if you are running it over top of an existing VM (not creating it from scratch), an error appears during the 'Extract and install Terraform' task:Finally, there were a lot of deprecation warnings generated while building the
angularjscontainer.Running tests or server Running tests or the server with the
-Wallflag inside the Django container also results in a few errors I did not address:A lot of
django.utils.translation.ugettext_lazy() is deprecated in favor of django.utils.translation.gettext_lazy()warnings; these are a result of us keepingdjango-localflavorat version 2.2.This goes back to an error I’ve seen before within
boto3, which this thread seems to describe as just noise. I did double check that all<file>.opens are either preceded bywithor followed by<file>.close()and then left it as is.Seems to be an issue with the
pastmodule.Testing Instructions
./scripts/setupto apply necessary migrations./scripts/serverand then navigate to http://localhost:9301/admin#/admin/analysis-jobs/docker-compose run --rm --entrypoint bash djangoand then runpython3 -Wall manage.py test --noinputpython3 -Wall manage.py runserverand verify that:Checklist
Resolves #876 & #877