Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update to Django 3.0 #346

Merged
merged 8 commits into from
Jul 15, 2020
Merged

Update to Django 3.0 #346

merged 8 commits into from
Jul 15, 2020

Conversation

MarkKoz
Copy link
Member

@MarkKoz MarkKoz commented Mar 21, 2020 •
edited
Loading

Resolves #344

Still needs to be tested extensively. If you have ideas for what should be tested, leave a comment.

MarkKoz added 3 commits March 21, 2020 11:08
@MarkKoz
Update to Django 3.0
f84ec78
@MarkKoz
Use basename instead of base_name with DRF router
e772593 
base_name was deprecated in 3.9 and finally removed in 3.11.
@MarkKoz
Allow empty list for message embeds
145beb3 
By default, blank=False for ArrayFields but allow_empty=True for
ListField. Before DRF 3.10 there was a bug that ListField didn't respect
the default value of blank=False and thus created a ListField in the
serialiser with the default of allow_empty=True. We were relying on the
behaviour of that bug.

See encode/django-rest-framework#6597.
@MarkKoz MarkKoz added area: dependencies Related to package dependencies and management status: stalled Something is blocking further progress type: feature New feature or request priority: 2 - normal Normal Priority labels Mar 21, 2020
@sco1
Copy link
Contributor

sco1 commented Mar 24, 2020 •
edited
Loading

Making a note so we remember to investigate later: Per CVE-2020-6816, we may need to temporarily pin the bleach dependency. If we even use bleach?

bleach is a django-wiki requirement & is currently pinned by 0.6b1 to bleach>=3.1.0,<3.2. The current PyPI version of bleach is 3.1.3, so the current django-wiki pinning should be sufficient, and presumably they'll bump the requirement with the next beta so it might not be necessary at all.

@lemonsaurus
Copy link
Member

We do not currently use bleach, it's a subdependency of django-wiki but we've disabled all the features that relate to it.

@lemonsaurus lemonsaurus removed the status: stalled Something is blocking further progress label Jul 14, 2020
@lemonsaurus
Copy link
Member

I don't think this is blocked anymore. We should just get it merged.

@lemonsaurus lemonsaurus marked this pull request as ready for review July 14, 2020 12:07
@lemonsaurus lemonsaurus requested a review from a team as a code owner July 14, 2020 12:07
@ghost ghost added the needs 2 approvals label Jul 14, 2020
kosayoda
kosayoda approved these changes Jul 14, 2020
View reviewed changes
Copy link
Member

@kosayoda kosayoda left a comment

Choose a reason for hiding this comment

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

Works fine during testing after merging master and running migrations. Nice!

@kosayoda kosayoda self-requested a review July 14, 2020 16:20
@kosayoda
Copy link
Member

Okay I should not be testing late at night.
I got an error running tests.

FAIL: test_supports_multi_creation (pydis_site.apps.api.tests.test_users.CreationTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/kosa/Documents/code/python/PythonDiscord/test/site/pydis_site/apps/api/tests/test_users.py", line 91, in test_supports_multi_creation
    self.assertEqual(response.status_code, 201)
AssertionError: 400 != 201

I'll try and figure out the issue if it's still there tomorrow.

@MarkKoz
Copy link
Member Author

MarkKoz commented Jul 14, 2020

@lemonsaurus django-crispy-bulma still has broken releases. python-discord/django-crispy-bulma#34 should fix it, but it's stale. Furthermore, someone did report some issues with the library python-discord/django-crispy-bulma#31 but I think those issues do not manifest themselves in how we're using the library on our site.

@lemonsaurus
Copy link
Member

Yeah. I'll see if I can address that stuff this week, now that I finally have some time. I think fixing all the crispy bulma stuff, then getting this merged, and then getting whitelisting merged seems like the right order of business.

MarkKoz added 4 commits July 14, 2020 13:08
@MarkKoz
Merge branch 'master' into feat/deps/344/django-3.0
2a5bd32
@MarkKoz
Update dependency pins
04806b3 
* Use release tag for django-crispy-bulma. This will yet again be
  changed once it's released on PyPI.
* Use PyPI release for wiki
@MarkKoz
Merge migrations
fbd00e1
@MarkKoz
Allow empty list for user roles
9208e0a 
This is the same issue as 145beb3.
See that commit for more information.
@MarkKoz
Copy link
Member Author

MarkKoz commented Jul 14, 2020

@kosayoda Thanks, I've fixed that bug now. The model was not allowing an empty list for a field. I'm worried we may have this bug elsewhere but don't test for it. I can't think of anything at the moment though. If you want info on the bug, see the message for 145beb3.

@lemonsaurus lemonsaurus mentioned this pull request Jul 14, 2020
Merged
@MarkKoz
Copy link
Member Author

MarkKoz commented Jul 15, 2020

We don't use ArrayField anywhere else so maybe we're fine.

@ghost ghost removed the needs 1 approval label Jul 15, 2020
@lemonsaurus lemonsaurus merged commit 1f4c840 into master Jul 15, 2020
@lemonsaurus lemonsaurus deleted the feat/deps/344/django-3.0 branch July 15, 2020 10:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kosayoda kosayoda kosayoda approved these changes

@lemonsaurus lemonsaurus lemonsaurus approved these changes

Assignees
No one assigned
Labels
area: dependencies Related to package dependencies and management priority: 2 - normal Normal Priority type: feature New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update to Django 3.0
4 participants
@MarkKoz @sco1 @lemonsaurus @kosayoda