Upgrade to Django 2.2. #1830

luzfcb · 2021-08-01T20:47:55Z

Fix #1819

Replaced the os.environ usage by python-decouple
Added the env_sample file with the environment variables that is available to use. If a file named .env exists on the project root path, the python-decouple will read. Note: Environment variables have precedence over the python-decouple config files.
Upgraded to Django 2.2.
Updated dependencies for django 2.2 compatibility
Synced django manage.py implementation with upstream
Added the apps.py files for all modules that have migrations, template tags, or management commands
Use path() or re_path() instead of url()
Use gettext_lazy instead of ugettext_lazy
Renamed filter_fields to filterset_fields and filter_class to filterset_class to be compatible with the new version of django-filter
Removed W605 invalid escape sequence \ in events/tests/test_importer.py (related to https://docs.python.org/3/whatsnew/3.6.html#deprecated-python-behavior)
Fix factory_boy DjangoModelFactory imports
Fix url_name function shadowing on pydotorg/context_processors.py
Added FORM_RENDERER = 'django.forms.renderers.DjangoTemplates' to pydotorg/settings/base.py
Added the obj=None argument on sponsors.admin.SponsorBenefitInline.has_add_permission method
Renamed base_name to basename on pydotorg/urls_api.py to be compatible with the new version of django-restframework
Added a missing migration to the added_by_user field of SponsorBenefit model that that Django <=2.0 cannot detect. This migration only includes blank=True in the migration and does not touch the database, which means do NOT generate SQL.
Load static templatetag instead of admin_static.
Load static templatetag instead of staticfiles.
Replace NullBooleanField() by BooleanField(null=True). Note: The migration will not generate an SQL. Django 2.1 release notes recommend this change but only in Django 3.1 NullBooleanField is effectively marked as deprecated.
Replaced django-easy-pdf by django-easy-pdf3 (use BytesIO from io and not from six)

TODO:

Upgrade django-allauth
Migrate to sentry-sdk (raven is deprecated)
Check if it is possible to generate the OpenAPI schema and after using it as a source for schemathesis generate and run tests and thus ensure that there are no update errors (at least in the underlying code that can be executed via API)

Django 3.2 TODO:

Note: The code as it is in this pull-request is already capable of running the test suite with Django 3.2

Add DEFAULT_AUTO_FIELD = 'django.db.models.AutoField' to pydotorg/settings/base.py to avoid unwanted migrations
Replace from django.contrib.postgres.fields import JSONField by from django.db.models import JSONField on community/models.py and deal with the migrations.
Remove default_app_config variable from all __init__.py files of all apps.

--

Pull-request sponsored by

Need a team for your project? Let's talk

luzfcb · 2021-08-02T02:30:28Z

@berinhard @ewdurbin I have no idea the cause of this error. https://app.travis-ci.com/github/python/pythondotorg/builds/234219521#L1327-L1429
Any suggestion? Any help is welcome.

.gitignore

.pre-commit-config.yaml

berinhard

@luzfcb thanks again for opening such an important PR. I noticed that there are 3 tests failing but I didn't have the opportunity to run then locally to investigate the problem. As soon as I finish with other priorities, I'll investigate them. If you want to pair on them, I'll be happy to do so. I'm usually available during morning time.

base-requirements.txt

.pre-commit-config.yaml

base-requirements.txt

env_sample

events/tests/test_importer.py

users/models.py

users/migrations/0014_auto_20210801_2332.py

sponsors/migrations/0031_auto_20210801_2038.py

luzfcb · 2021-08-11T09:03:03Z

@berinhard I updated the description, made some requested changes, and rebased with the main branch. Can review again?

luzfcb · 2021-08-11T09:11:54Z

I added a test case to check for missing migrations.
If this test fails, something like this will be shown:

```python
FAIL: test_missing_migrations (pydotorg.tests.test_migrations.MissingMigrationTestCase)
Tests if there are any changes left in the Django models for which the migration script was not generated.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/build/python/pythondotorg/pydotorg/tests/test_migrations.py", line 24, in test_missing_migrations
    self.assertFalse(
AssertionError: {'sponsors': [<Migration sponsors.0031_auto_20210811_0901>]} is not false : Missing migrations were detected on the following apps: sponsors. Run `python3 manage.py makemigrations` to generate new ones.
```

https://app.travis-ci.com/github/python/pythondotorg/builds/235045671#L1655

yes, I know there is `python manage.py makemigrations --dry-run --check`, but that way, we ensure there will never be a missing migration even if we forget to run the `makemigrations --dry-run --check`

I created a separate pull-request for this issue #1839

luzfcb · 2021-08-21T19:14:05Z

templates/components/navigation-widget.html

-
+{% elif URL_NAME %}
    {% sitetree_tree from URL_NAME template "sitetree/sidebar_menu_root.html" %}
+{% endif %}


In theory, this makes the 3 tests start to pass
https://app.travis-ci.com/github/python/pythondotorg/builds/235762782#L1343-L1648

I'm not sure this is the correct way to fix this.

Anyway, thanks @lucianoratamero for helping me debug this error.

From what I could understand, this context processor, for some reason, wasn't raising the Resolver404 exception for db-routes such as is happening with tests related to Page or Release objects.

I also checked the PageView implementation to confirm if it wasn't doing something unexpected but I didn't see anything out of place.

I tested 2 type of dynamic pages to check what would break if the path wasn't configured correctly. If I access an URL (/lorem-ipsum/) mapped to a Page object with the URL not being listed under the sitetree, I can see the content but without the navigation links as you can see here:

If update this same object to have a path which is listed under the site tree, such as /about/gettingstarted/, the user sees both the page content and the navigation links as expected. Here's the print screen:

So, I think this fix is a good one 👍

luzfcb · 2021-08-21T19:14:55Z

@berinhard @ewdurbin this pull request is ready for review.

berinhard

@luzfcb the PR #1848 replaced travis by github actions and this is resulting in conflicts. Can you update .github/workflows/ci.yml file instead?

@ewdurbin I tested it locally and everything seems to be good so far. But since you have a more "trained" pair of eyes on how the site should look like, it would be nice to have your review and also your comments on the still open discussions.

berinhard · 2021-08-24T15:08:47Z

templates/components/navigation-widget.html

-
+{% elif URL_NAME %}
    {% sitetree_tree from URL_NAME template "sitetree/sidebar_menu_root.html" %}
+{% endif %}


From what I could understand, this context processor, for some reason, wasn't raising the Resolver404 exception for db-routes such as is happening with tests related to Page or Release objects.

I also checked the PageView implementation to confirm if it wasn't doing something unexpected but I didn't see anything out of place.

I tested 2 type of dynamic pages to check what would break if the path wasn't configured correctly. If I access an URL (/lorem-ipsum/) mapped to a Page object with the URL not being listed under the sitetree, I can see the content but without the navigation links as you can see here:

If update this same object to have a path which is listed under the site tree, such as /about/gettingstarted/, the user sees both the page content and the navigation links as expected. Here's the print screen:

So, I think this fix is a good one 👍

luzfcb · 2021-08-24T15:39:03Z

@berinhard

@luzfcb the PR #1848 replaced travis by github actions and this is resulting in conflicts. Can you update .github/workflows/ci.yml file instead?

Fixed.

luzfcb · 2021-08-24T20:08:06Z

@berinhard I pushed a fix for PostgreSQL running in GitHub Actions

luzfcb · 2021-08-30T13:17:46Z

@berinhard @ewdurbin this pull-request is ready for review and merge.

ewdurbin

Spun this up and smoke tested it with a copy of the prod db and all seems well! There's one migration in the sponsors app that needs to be updated and then we can ship it!

sponsors/migrations/0037_add_added_by_user_sponsorbenefit.py

Update dependencies for django 2.2 compatibility

luzfcb · 2021-09-08T10:56:56Z

@ewdurbin I made the requested changes. It's ready for review. As it's the first time I've contributed to this project, Github Action workflow needs manual approval again to run

ewdurbin

Thank you for this effort!

ewdurbin · 2021-09-08T14:42:53Z

Deploying now! I'll be keeping a close eye on the application through the rest of the day <3

ewdurbin · 2021-09-13T15:55:21Z

Thanks so much @luzfcb! This has been rolling smoothly now for days!

luzfcb force-pushed the django2.2 branch 7 times, most recently from 580250d to d302197 Compare August 2, 2021 02:29

luzfcb marked this pull request as ready for review August 2, 2021 02:41

luzfcb commented Aug 2, 2021

View reviewed changes

.gitignore Show resolved Hide resolved

luzfcb commented Aug 2, 2021

View reviewed changes

.pre-commit-config.yaml Outdated Show resolved Hide resolved

luzfcb marked this pull request as draft August 2, 2021 14:03

berinhard reviewed Aug 10, 2021

View reviewed changes

luzfcb force-pushed the django2.2 branch from d302197 to 3fd5efb Compare August 11, 2021 08:58

luzfcb requested a review from berinhard August 11, 2021 09:00

luzfcb force-pushed the django2.2 branch from 3fd5efb to 106e49c Compare August 11, 2021 09:07

luzfcb force-pushed the django2.2 branch 4 times, most recently from 7785b5d to 9d8449b Compare August 21, 2021 19:10

luzfcb commented Aug 21, 2021

View reviewed changes

luzfcb marked this pull request as ready for review August 21, 2021 19:14

berinhard approved these changes Aug 24, 2021

View reviewed changes

luzfcb force-pushed the django2.2 branch 2 times, most recently from 044928a to 9928e1b Compare August 24, 2021 15:27

luzfcb force-pushed the django2.2 branch from f911550 to 2e4006c Compare August 24, 2021 19:48

luzfcb force-pushed the django2.2 branch from 2e4006c to e650ddc Compare August 30, 2021 13:13

luzfcb force-pushed the django2.2 branch from e650ddc to 1cfee93 Compare August 30, 2021 13:29

ewdurbin requested changes Sep 7, 2021

View reviewed changes

sponsors/migrations/0037_add_added_by_user_sponsorbenefit.py Outdated Show resolved Hide resolved

luzfcb added 2 commits September 8, 2021 07:52

Upgrade to Django 2.2.

6b218e5

Update dependencies for django 2.2 compatibility

Fix Github Action PostgreSQL config

817fc24

luzfcb force-pushed the django2.2 branch from 1cfee93 to 817fc24 Compare September 8, 2021 10:52

ewdurbin approved these changes Sep 8, 2021

View reviewed changes

berinhard mentioned this pull request Sep 8, 2021

Upgrade to Django 3.2 LTS version #1860

Closed

ewdurbin merged commit 8a02c10 into python:main Sep 8, 2021

JacobCoffee mentioned this pull request Sep 12, 2024

Bump django-filter from 1.1.0 to 2.4.0 #1640

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade to Django 2.2. #1830

Upgrade to Django 2.2. #1830

luzfcb commented Aug 1, 2021 •

edited

Loading

luzfcb commented Aug 2, 2021 •

edited

Loading

berinhard left a comment

luzfcb commented Aug 11, 2021

luzfcb commented Aug 11, 2021 •

edited

Loading

luzfcb Aug 21, 2021

berinhard Aug 24, 2021

luzfcb commented Aug 21, 2021

berinhard left a comment

berinhard Aug 24, 2021

luzfcb commented Aug 24, 2021

luzfcb commented Aug 24, 2021

luzfcb commented Aug 30, 2021

ewdurbin left a comment

luzfcb commented Sep 8, 2021

ewdurbin left a comment

ewdurbin commented Sep 8, 2021

ewdurbin commented Sep 13, 2021

Upgrade to Django 2.2. #1830

Upgrade to Django 2.2. #1830

Conversation

luzfcb commented Aug 1, 2021 • edited Loading

TODO:

Django 3.2 TODO:

Pull-request sponsored by

luzfcb commented Aug 2, 2021 • edited Loading

berinhard left a comment

Choose a reason for hiding this comment

luzfcb commented Aug 11, 2021

luzfcb commented Aug 11, 2021 • edited Loading

luzfcb Aug 21, 2021

Choose a reason for hiding this comment

berinhard Aug 24, 2021

Choose a reason for hiding this comment

luzfcb commented Aug 21, 2021

berinhard left a comment

Choose a reason for hiding this comment

berinhard Aug 24, 2021

Choose a reason for hiding this comment

luzfcb commented Aug 24, 2021

luzfcb commented Aug 24, 2021

luzfcb commented Aug 30, 2021

ewdurbin left a comment

Choose a reason for hiding this comment

luzfcb commented Sep 8, 2021

ewdurbin left a comment

Choose a reason for hiding this comment

ewdurbin commented Sep 8, 2021

ewdurbin commented Sep 13, 2021

luzfcb commented Aug 1, 2021 •

edited

Loading

luzfcb commented Aug 2, 2021 •

edited

Loading

luzfcb commented Aug 11, 2021 •

edited

Loading