-
Notifications
You must be signed in to change notification settings - Fork 850
Retry peeruserimport task on Database or connection errors #13821
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
base: develop
Are you sure you want to change the base?
Changes from all commits
c5e1a8e
e6879c6
eb53ac5
57ba0d3
486c896
58597eb
579b9e7
82167f7
c6c00e4
b579ed4
5a8b0cd
283ffde
1a2f204
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -142,17 +142,14 @@ def test_restore_from_latest(): | |
|
|
||
| # Restore it into a new test database setting | ||
| with override_settings(DATABASES=MOCK_DATABASES): | ||
| from django import db | ||
|
|
||
| # Destroy current connections and create new ones: | ||
| db.connections.close_all() | ||
| db.connections = db.ConnectionHandler() | ||
| call_command("dbrestore", "-l") | ||
| # Test that the user has been restored! | ||
| assert ( | ||
| Facility.objects.filter(name="test latest", kind=FACILITY).count() == 1 | ||
| ) | ||
| _clear_backups(default_backup_folder()) | ||
| with patch("django.db.connections", ConnectionHandler()): | ||
| call_command("dbrestore", "-l") | ||
| # Test that the user has been restored! | ||
| assert ( | ||
| Facility.objects.filter(name="test latest", kind=FACILITY).count() | ||
| == 1 | ||
| ) | ||
| _clear_backups(default_backup_folder()) | ||
|
|
||
|
|
||
| @pytest.mark.django_db(transaction=True) | ||
|
|
@@ -203,9 +200,6 @@ def test_restore_from_file_to_file(): | |
| dest_folder = tempfile.mkdtemp() | ||
| # Purposefully destroy the connection pointer, which is the default | ||
| # state of an unopened connection | ||
| from django import db | ||
|
|
||
| db.connections["default"].connection = None | ||
|
Comment on lines
-206
to
-208
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. idem, will instead rely on the django.db.connections patch. But not sure if this may cause false positives. |
||
| backup = dbbackup(kolibri.__version__, dest_folder=dest_folder) | ||
|
|
||
| # Restore it into a new test database setting | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,7 @@ def register_task( | |
| permission_classes=None, | ||
| long_running=False, | ||
| status_fn=None, | ||
| retry_on=None, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good job avoiding a classic Python gotcha! (passing mutable values as default arguments, such as |
||
| ): | ||
| """ | ||
| Registers the decorated function as task. | ||
|
|
@@ -36,6 +37,7 @@ def register_task( | |
| permission_classes=permission_classes, | ||
| long_running=long_running, | ||
| status_fn=status_fn, | ||
| retry_on=retry_on, | ||
| ) | ||
|
|
||
| return RegisteredTask( | ||
|
|
@@ -49,4 +51,5 @@ def register_task( | |
| permission_classes=permission_classes, | ||
| long_running=long_running, | ||
| status_fn=status_fn, | ||
| retry_on=retry_on, | ||
| ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| # Initial migration for the jobs table. This model/index creation should be | ||
| # skipped if the table was previously created by SQLAlchemy. | ||
| from django.db import migrations | ||
| from django.db import models | ||
|
|
||
| from kolibri.core.tasks.operations import AddIndexIfNotExists | ||
| from kolibri.core.tasks.operations import CreateModelIfNotExists | ||
|
|
||
|
|
||
| class Migration(migrations.Migration): | ||
|
|
||
| initial = True | ||
|
|
||
| dependencies = [] | ||
|
|
||
| operations = [ | ||
| CreateModelIfNotExists( | ||
| name="Job", | ||
| fields=[ | ||
| ("id", models.CharField(max_length=36, primary_key=True)), | ||
| ("state", models.CharField(max_length=20, db_index=True)), | ||
| ("func", models.CharField(max_length=200, db_index=True)), | ||
| ("priority", models.IntegerField(db_index=True)), | ||
| ("queue", models.CharField(max_length=50, db_index=True)), | ||
| ("saved_job", models.TextField()), | ||
| ("time_created", models.DateTimeField(null=True, blank=True)), | ||
| ("time_updated", models.DateTimeField(null=True, blank=True)), | ||
| ("interval", models.IntegerField(default=0)), | ||
| ("retry_interval", models.IntegerField(null=True, blank=True)), | ||
| ("repeat", models.IntegerField(null=True, blank=True)), | ||
| ("scheduled_time", models.DateTimeField(null=True, blank=True)), | ||
| ( | ||
| "worker_host", | ||
| models.CharField(max_length=100, null=True, blank=True), | ||
| ), | ||
| ( | ||
| "worker_process", | ||
| models.CharField(max_length=50, null=True, blank=True), | ||
| ), | ||
| ( | ||
| "worker_thread", | ||
| models.CharField(max_length=50, null=True, blank=True), | ||
| ), | ||
| ("worker_extra", models.TextField(null=True, blank=True)), | ||
| ], | ||
| options={ | ||
| "db_table": "jobs", | ||
| }, | ||
| ), | ||
| AddIndexIfNotExists( | ||
| model_name="job", | ||
| index=models.Index( | ||
| fields=["queue", "scheduled_time"], name="queue__scheduled_time" | ||
| ), | ||
| ), | ||
| ] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| # Generated by Django 3.2.25 on 2025-10-15 21:14 | ||
| from django.db import migrations | ||
| from django.db import models | ||
|
|
||
|
|
||
| class Migration(migrations.Migration): | ||
|
|
||
| dependencies = [ | ||
| ("kolibritasks", "0001_initial"), | ||
| ] | ||
|
|
||
| operations = [ | ||
| migrations.AddField( | ||
| model_name="job", | ||
| name="max_retries", | ||
| field=models.IntegerField(blank=True, null=True), | ||
| ), | ||
| migrations.AddField( | ||
| model_name="job", | ||
| name="retries", | ||
| field=models.IntegerField(blank=True, null=True), | ||
| ), | ||
| ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have removed these
db.connectionsoverrides and have used thepatch("django.db.connections"instead. These overrides were having some side effects on the job tests that involved having multiple threads, and it was messing things up in the teardown process.However, not sure if removing these lines may cause somehow a false positive in the test.