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.

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

Django to 3.0 #1730

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from
Open

Django to 3.0 #1730

wants to merge 6 commits into from

Conversation

bbearce
Copy link
Collaborator

@bbearce bbearce commented Jan 16, 2025

Description

Consumer needed a new library "asgiref" versus "channels.db" for using the method "sync_to_async".

Issues this PR resolves

Dependabot wanted to upgrade Django to the absolute latest which is too big to do yet. This however upgrades django from 2.2.28 to 3.0. This was a large change but not too large.

A checklist for hand testing

  • Rebuild django and site_worker or really all images with docker compose up -d build
  • Run tests

Checklist

  • Code review by me
  • Hand tested by me
  • I'm proud of my work
  • Code review by reviewer
  • Hand tested by reviewer
  • CircleCi tests are passing
  • Ready to merge

@bbearce bbearce changed the title consumer async and template static loading changes Django to 3.0 Jan 16, 2025
@bbearce
Copy link
Collaborator Author

bbearce commented Jan 16, 2025

Current problem is that running:

docker compose exec django py.test -m "not e2e"

Is failing in file src/apps/api/tests/test_competitions.py in test test_adding_organizer_creates_accepted_participant. I see that the issues is that

        assert CompetitionParticipant.objects.filter(
            user=self.other_user,
            competition=self.comp,
            status=CompetitionParticipant.APPROVED
        ).count() == 1

produces a count of 1 in django 2.28.0 and in 3.0 I get not returned results:

3.0:

> /app/src/apps/api/tests/test_competitions.py(52)test_adding_organizer_creates_accepted_participant()
-> assert CompetitionParticipant.objects.filter(
(Pdb) CompetitionParticipant.objects.filter(user=self.other_user,competition=self.comp,status=CompetitionParticipant.APPROVED)
<QuerySet []>
(Pdb) 

2.2.28:

> /app/src/apps/api/tests/test_competitions.py(52)test_adding_organizer_creates_accepted_participant()
-> assert CompetitionParticipant.objects.filter(
(Pdb) CompetitionParticipant.objects.filter(user=self.other_user,competition=self.comp,status=CompetitionParticipant.APPROVED)
<QuerySet [<CompetitionParticipant: (238) - User: other_user in Competition: Competition 39>]>
(Pdb) 

@bbearce
Copy link
Collaborator Author

bbearce commented Jan 16, 2025

Ok so this line:

resp = self.client.put(url, data=json.dumps(data), content_type="application/json")

is supposed to edit the data object which is holding an updated "colaborators" list with the 'other_user' pk. It seems like it is not doing that.

Django 2.2.28:
image

Django 3.0
image

So the new version isn't adding 'other_user' to table 'competitions_competitionparticipant' based on PUT request.

@bbearce bbearce closed this Jan 16, 2025
@bbearce bbearce reopened this Jan 16, 2025
@ObadaS
Copy link
Collaborator

ObadaS commented Jan 16, 2025

Have you tried using different version of PostgresSQL and Django ?
For example, Django 3.0.14 is the last version before they switch to 3.1, maybe Django 3.0 has a regression or something that makes it bug with the version of PostgresSQL we are using (for some commands at lest since the rest seems to be working fine)

@bbearce
Copy link
Collaborator Author

bbearce commented Jan 16, 2025

So I found out that in Django 2.2.28 and below many to many relationships when altered auto updated the complex entities they are related to. People complained about too much magic in the database operations and so the bug is that we need to create CompetitionParticipant records for folks marked as collaborators or admins or a challenge from the Edit menu. It happens automatically in django 2 but not in 3 on purpose.

There is another bug I'm working on now related to solutions in a task. In the automated tests, they create a task with submission and solution and something about triangulating the two of them in the TaskViewSet in def get_queryset is problematic. Line ~66:

qs = qs.annotate(validated=Subquery(task_validate_qs.values('pk')[:1]))

throws an error.

@ObadaS - I haven't ruled anything out. I guess this is worth toggling.

@bbearce
Copy link
Collaborator Author

bbearce commented Jan 18, 2025

@ObadaS :

Tried:

  • image: postgres:13-alpine
  • image: postgres:14-alpine
  • image: postgres:15-alpine

Still has the same error:

==================================================================================================================== short test summary info =====================================================================================================================
FAILED src/apps/api/tests/test_tasks.py::TestTasks::test_task_shown_as_validated_properly - django.db.utils.ProgrammingError: syntax error at or near ""tasks_solution""
================================================================================================================ 1 failed, 32 warnings in 13.38s =================================================================================================================

Used this structure to try diff postgres's:

DB_NAME=postgres
DB_USERNAME=postgres
DB_PASSWORD=postgres
DUMP_NAME=postgres:12-alpine

docker exec codabench-db-1 bash -c "PGPASSWORD=$DB_PASSWORD pg_dump -Fc -U $DB_USERNAME $DB_NAME > /app/backups/$DUMP_NAME.dump"

# Also to be sure
# sudo cp -r /home/bbearce/Documents/codabench/var/postgres /home/bbearce/Documents/codabench/var/postgres_orig
sudo rm -r /home/bbearce/Documents/codabench/var/postgres/*

docker compose down
# Change version of postgres image under service 'db'
docker compose up -d
docker compose logs -f db

docker compose exec db bash
dropdb $DB_NAME -U $DB_USERNAME
createdb $DB_NAME -U $DB_USERNAME
pg_restore -U $DB_USERNAME -d $DB_NAME -1 /app/backups/postgres:12-alpine.dump

It didn't work. There might be something I'm missing and maybe I can keep trying images but I have a hunch the logic is the issue. Going to keep going down that road for now.

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