Skip to content

Commit

Permalink
fix: submit the create_enterprise_enrollment task with a configurable…
Browse files Browse the repository at this point in the history
… countdown

Also removes python 3.8 from CI matrix.
  • Loading branch information
iloveagent57 committed Jun 14, 2024
1 parent a178e8a commit 007abaf
Show file tree
Hide file tree
Showing 9 changed files with 36 additions and 23 deletions.
3 changes: 1 addition & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ jobs:
fail-fast: false
matrix:
python-version:
- '3.8'
- '3.11'
toxenv: [quality, docs, django42-celery53, django42-pii-annotations]
env:
Expand All @@ -39,7 +38,7 @@ jobs:
TOXENV: ${{ matrix.toxenv }}
run: tox
- name: Run code coverage
if: matrix.python-version == '3.8' && matrix.toxenv == 'django42-celery53'
if: matrix.python-version == '3.11' && matrix.toxenv == 'django42-celery53'
uses: codecov/codecov-action@v4
with:
flags: unittests
Expand Down
8 changes: 6 additions & 2 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,16 @@ Unreleased
----------
* nothing unreleased

[4.20.5]
--------
* fix: submit the ``create_enterprise_enrollment`` task with a configurable countdown

[4.20.4]
---------
--------
* feat: Populates ``learner_portal_sidebar_content`` in ``EnterpriseCustomer`` and removes references to old fields.

[4.20.3]
---------
--------
* feat: Makes ``career_engagement_network_message`` field nullable in ``EnterpriseCustomer``.

[4.20.2]
Expand Down
2 changes: 1 addition & 1 deletion enterprise/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
Your project description goes here.
"""

__version__ = "4.20.4"
__version__ = "4.20.5"
4 changes: 2 additions & 2 deletions enterprise/admin/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -770,8 +770,8 @@ def clean(self):
# `start_date` must always come before the `expiration_date`
raise ValidationError({'expiration_date': ['Expiration date should be after start date.']})
admin_notification = AdminNotification.objects.filter(
Q(start_date__range=(start_date, expiration_date)) | # pylint: disable=unsupported-binary-operation
Q(expiration_date__range=(start_date, expiration_date)) | # pylint: disable=unsupported-binary-operation
Q(start_date__range=(start_date, expiration_date)) |
Q(expiration_date__range=(start_date, expiration_date)) |
Q(start_date__lt=start_date, expiration_date__gt=expiration_date)
).exclude(
pk=self.instance.id
Expand Down
1 change: 0 additions & 1 deletion enterprise/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,6 @@ def has_explicit_access_to_reporting_api(user, obj):
)


# pylint: disable=unsupported-binary-operation
rules.add_perm('enterprise.can_access_admin_dashboard',
has_implicit_access_to_dashboard | has_explicit_access_to_dashboard)

Expand Down
18 changes: 14 additions & 4 deletions enterprise/signals.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

from logging import getLogger

from django.conf import settings
from django.db import transaction
from django.db.models.signals import post_delete, post_save, pre_save
from django.dispatch import receiver
Expand Down Expand Up @@ -49,6 +50,10 @@
SAPSuccessFactorsEnterpriseCustomerConfiguration,
]

# Default number of seconds to use as task countdown
# if not otherwise specified via Django settings.
DEFAULT_COUNTDOWN = 3


@disable_for_loaddata
def handle_user_post_save(sender, **kwargs): # pylint: disable=unused-argument
Expand Down Expand Up @@ -385,6 +390,10 @@ def create_enterprise_enrollment_receiver(sender, instance, **kwargs): # pyl
instance.course_id,
)

# Number of seconds to tell celery to wait before the `create_enterprise_enrollment`
# task should begin execution.
countdown = getattr(settings, 'CREATE_ENTERPRISE_ENROLLMENT_TASK_COUNTDOWN', DEFAULT_COUNTDOWN)

def submit_task():
"""
In-line helper to run the create_enterprise_enrollment task on commit.
Expand All @@ -393,10 +402,11 @@ def submit_task():
"User %s is an EnterpriseCustomerUser. Spinning off task to check if course is within User's "
"Enterprise's EnterpriseCustomerCatalog."
), user_id)
create_enterprise_enrollment.delay(
str(instance.course_id),
ecu.id,
)
task_args = (str(instance.course_id), ecu.id)
# Submit the task with a countdown to help avoid possible race-conditions/deadlocks
# due to external processes that read or write the same
# records the task tries to read or write.
create_enterprise_enrollment.apply_async(task_args, countdown=countdown)

# This receiver might be executed within a transaction that creates an ECE record.
# Ensure that the task is only submitted after a commit tasks place, because
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ def _check_matched_content_updated_at(
)

content_query.add(
Q(remote_errored_at__lt=self.LAST_24_HRS) | Q(remote_errored_at__isnull=True) | # pylint: disable=unsupported-binary-operation
Q(remote_errored_at__lt=self.LAST_24_HRS) | Q(remote_errored_at__isnull=True) |
Q(remote_errored_at__lt=self.enterprise_customer.modified),
Q.AND
)
Expand Down Expand Up @@ -414,7 +414,7 @@ def _check_matched_content_to_delete(self, enterprise_customer_catalog, items):
)

past_content_query.add(
Q(remote_errored_at__lt=self.LAST_24_HRS) | Q(remote_errored_at__isnull=True) | # pylint: disable=unsupported-binary-operation
Q(remote_errored_at__lt=self.LAST_24_HRS) | Q(remote_errored_at__isnull=True) |
Q(remote_errored_at__lt=self.enterprise_customer.modified), Q.AND)
past_content = ContentMetadataItemTransmission.objects.filter(
past_content_query).first()
Expand Down
10 changes: 5 additions & 5 deletions integrated_channels/integrated_channel/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ def fetch_orphaned_content_audits(self):
enterprise_customer=self.enterprise_customer,
remote_deleted_at__isnull=True,
remote_created_at__isnull=False,
).filter(~non_existent_catalogs_filter | null_catalogs_filter) # pylint: disable=unsupported-binary-operation
).filter(~non_existent_catalogs_filter | null_catalogs_filter)

def update_content_synced_at(self, action_happened_at, was_successful):
"""
Expand Down Expand Up @@ -723,7 +723,7 @@ def deleted_transmissions(cls, enterprise_customer, plugin_configuration_id, int
remote_deleted_at__isnull=False,
)
query.add(
Q(remote_errored_at__lt=LAST_24_HRS) # pylint: disable=unsupported-binary-operation
Q(remote_errored_at__lt=LAST_24_HRS)
| Q(remote_errored_at__isnull=True)
| Q(remote_errored_at__lt=enterprise_customer.modified), Q.AND)
return ContentMetadataItemTransmission.objects.filter(query)
Expand Down Expand Up @@ -760,7 +760,7 @@ def incomplete_create_transmissions(
api_response_status_code__gte=400,
)
in_db_but_failed_to_send_query.add(
Q(remote_errored_at__lt=LAST_24_HRS) | Q(remote_errored_at__isnull=True) | # pylint: disable=unsupported-binary-operation
Q(remote_errored_at__lt=LAST_24_HRS) | Q(remote_errored_at__isnull=True) |
Q(remote_errored_at__lt=enterprise_customer.modified), Q.AND)
in_db_but_unsent_query.add(in_db_but_failed_to_send_query, Q.OR)
return ContentMetadataItemTransmission.objects.filter(in_db_but_unsent_query)
Expand All @@ -787,7 +787,7 @@ def incomplete_update_transmissions(
api_response_status_code__gte=400,
)
in_db_but_failed_to_send_query.add(
Q(remote_errored_at__lt=LAST_24_HRS) | Q(remote_errored_at__isnull=True) | # pylint: disable=unsupported-binary-operation
Q(remote_errored_at__lt=LAST_24_HRS) | Q(remote_errored_at__isnull=True) |
Q(remote_errored_at__lt=enterprise_customer.modified), Q.AND)
return ContentMetadataItemTransmission.objects.filter(in_db_but_failed_to_send_query)

Expand All @@ -812,7 +812,7 @@ def incomplete_delete_transmissions(
api_response_status_code__gte=400,
)
in_db_but_failed_to_send_query.add(
Q(remote_errored_at__lt=LAST_24_HRS) # pylint: disable=unsupported-binary-operation
Q(remote_errored_at__lt=LAST_24_HRS)
| Q(remote_errored_at__isnull=True)
| Q(remote_errored_at__lt=enterprise_customer.modified), Q.AND)
return ContentMetadataItemTransmission.objects.filter(in_db_but_failed_to_send_query)
Expand Down
9 changes: 5 additions & 4 deletions tests/test_enterprise/test_signals.py
Original file line number Diff line number Diff line change
Expand Up @@ -843,7 +843,7 @@ def setUp(self):
self.non_enterprise_user = UserFactory(id=999, email='user999@example.com')
super().setUp()

@mock.patch('enterprise.tasks.create_enterprise_enrollment.delay')
@mock.patch('enterprise.tasks.create_enterprise_enrollment.apply_async')
def test_receiver_calls_task_if_ecu_exists(self, mock_task):
"""
Receiver should call a task
Expand All @@ -863,11 +863,12 @@ def test_receiver_calls_task_if_ecu_exists(self, mock_task):
'created': True,
}

with self.captureOnCommitCallbacks(execute=True):
with self.captureOnCommitCallbacks(execute=True), \
override_settings(CREATE_ENTERPRISE_ENROLLMENT_TASK_COUNTDOWN=42):
create_enterprise_enrollment_receiver(sender, instance, **kwargs)
mock_task.assert_called_once_with(str(instance.course_id), self.enterprise_customer_user.id)
mock_task.assert_called_once_with((str(instance.course_id), self.enterprise_customer_user.id), countdown=42)

@mock.patch('enterprise.tasks.create_enterprise_enrollment.delay')
@mock.patch('enterprise.tasks.create_enterprise_enrollment.apply_async')
def test_receiver_does_not_call_task_if_ecu_not_exists(self, mock_task):
"""
Receiver should NOT call a task
Expand Down

0 comments on commit 007abaf

Please sign in to comment.