-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add Django 4 compatibility #2
Changes from 6 commits
4b1e4c3
93ebddb
7939915
2a9c97e
edcd464
e23e3a2
7b99f08
6e13e2c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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. I believe this file is unused as it uses github/workflows for the test runners. |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -5,13 +5,13 @@ | |||||
|
||||||
class DatabaseLock(DjangoCronJobLock): | ||||||
""" | ||||||
Locking cron jobs with database. Its good when you have not parallel run and want to make sure 2 jobs won't be | ||||||
Locking cron jobs with database. It's good when you have not parallel run and want to make sure 2 jobs won't be | ||||||
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. "have not parallel run" doesn't read right? I'm not sure exactly what it means.
Suggested change
|
||||||
fired at the same time - which may happened when job execution is longer that job interval. | ||||||
""" | ||||||
|
||||||
@transaction.atomic | ||||||
def lock(self): | ||||||
lock, created = CronJobLock.objects.get_or_create(job_name=self.job_name) | ||||||
lock, created = CronJobLock.objects.select_for_update().get_or_create(job_name=self.job_name) | ||||||
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. Probably isn't needed for us, but good future proofing, resolves an issue listed here |
||||||
if lock.locked: | ||||||
return False | ||||||
else: | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
from django.conf import settings | ||
|
||
from django_common.helper import send_mail | ||
from django.core.mail import send_mail | ||
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. Fixes this issue |
||
|
||
from django_cron import CronJobBase, Schedule, get_class | ||
from django_cron.models import CronJobLog | ||
|
@@ -97,5 +97,5 @@ def get_send_mail_kwargs(self, cron_cls, failed_jobs): | |
return dict( | ||
subject=subject, message=message, | ||
from_email=self.config['CRON_FAILURE_FROM_EMAIL'], | ||
recipient_emails=self.config['CRON_FAILURE_EMAIL_RECIPIENTS'] | ||
recipient_list=self.config['CRON_FAILURE_EMAIL_RECIPIENTS'] | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -480,5 +480,5 @@ def test_uses_send_mail(self, mock_send_mail): | |
self.assertIn('ERROR!!!', kwargs['subject']) | ||
self.assertEquals('[email protected]', kwargs['from_email']) | ||
self.assertEquals( | ||
['[email protected]', '[email protected]'], kwargs['recipient_emails'] | ||
['[email protected]', '[email protected]'], kwargs['recipient_list'] | ||
) |
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.
Required due to actions/runner#1989