From cce8a53ba9e483a4f780708052fef6eb131dc654 Mon Sep 17 00:00:00 2001 From: Bill DeRusha <bderusha@C02X31WFJGH5.tld> Date: Fri, 24 Aug 2018 15:50:13 -0400 Subject: [PATCH] Add logging to signal and flag to mgmt cmd for credential grades --- .../management/commands/notify_credentials.py | 21 +++++- .../core/djangoapps/credentials/signals.py | 74 +++++++++++++++++-- openedx/core/djangoapps/programs/signals.py | 28 +++++++ 3 files changed, 113 insertions(+), 10 deletions(-) diff --git a/openedx/core/djangoapps/credentials/management/commands/notify_credentials.py b/openedx/core/djangoapps/credentials/management/commands/notify_credentials.py index 61d19a1c637..086ba62aa0d 100644 --- a/openedx/core/djangoapps/credentials/management/commands/notify_credentials.py +++ b/openedx/core/djangoapps/credentials/management/commands/notify_credentials.py @@ -140,6 +140,11 @@ class Command(BaseCommand): action='store_true', help='Use arguments from the NotifyCredentialsConfig model instead of the command line.', ) + parser.add_argument( + '--verbose', + action='store_true', + help='Run grade/cert change signal in verbose mode', + ) def get_args_from_database(self): """ Returns an options dictionary from the current NotifyCredentialsConfig model. """ @@ -198,11 +203,12 @@ class Command(BaseCommand): self.send_notifications(certs, grades, site_config=site_config, delay=options['delay'], - page_size=options['page_size']) + page_size=options['page_size'], + verbose=options['verbose']) log.info('notify_credentials finished') - def send_notifications(self, certs, grades, site_config=None, delay=0, page_size=0): + def send_notifications(self, certs, grades, site_config=None, delay=0, page_size=0, verbose=False): """ Run actual handler commands for the provided certs and grades. """ # First, do certs @@ -222,6 +228,7 @@ class Command(BaseCommand): 'course_key': cert.course_id, 'mode': cert.mode, 'status': cert.status, + 'verbose': verbose, } handle_course_cert_changed(**signal_args) handle_cert_change(**signal_args) @@ -238,7 +245,15 @@ class Command(BaseCommand): ) user = User.objects.get(id=grade.user_id) - send_grade_if_interesting(user, grade.course_id, None, None, grade.letter_grade, grade.percent_grade) + send_grade_if_interesting( + user, + grade.course_id, + None, + None, + grade.letter_grade, + grade.percent_grade, + verbose=verbose + ) def get_course_keys(self, courses): """ diff --git a/openedx/core/djangoapps/credentials/signals.py b/openedx/core/djangoapps/credentials/signals.py index aae6da7e71f..95a2a455508 100644 --- a/openedx/core/djangoapps/credentials/signals.py +++ b/openedx/core/djangoapps/credentials/signals.py @@ -42,15 +42,42 @@ def is_course_run_in_a_program(course_run_key): return False -def send_grade_if_interesting(user, course_run_key, mode, status, letter_grade, percent_grade): +def send_grade_if_interesting(user, course_run_key, mode, status, letter_grade, percent_grade, verbose=False): """ Checks if grade is interesting to Credentials and schedules a Celery task if so. """ + if verbose: + msg = "Starting send_grade_if_interesting with params: "\ + "user [{username}], "\ + "course_run_key [{key}], "\ + "mode [{mode}], "\ + "status [{status}], "\ + "letter_grade [{letter_grade}], "\ + "percent_grade [{percent_grade}], "\ + "verbose [{verbose}]"\ + .format( + username=getattr(user, 'username', None), + key=str(course_run_key), + mode=mode, + status=status, + letter_grade=letter_grade, + percent_grade=percent_grade, + verbose=verbose + ) + log.info(msg) # Avoid scheduling new tasks if certification is disabled. (Grades are a part of the records/cert story) if not CredentialsApiConfig.current().is_learner_issuance_enabled: + if verbose: + log.info("Skipping send grade: is_learner_issuance_enabled False") return # Avoid scheduling new tasks if learner records are disabled for this site. if not helpers.get_value_for_org(course_run_key.org, 'ENABLE_LEARNER_RECORDS', True): + if verbose: + log.info( + "Skipping send grade: ENABLE_LEARNER_RECORDS False for org [{org}]".format( + org=course_run_key.org + ) + ) return # Grab mode/status if we don't have them in hand @@ -61,23 +88,48 @@ def send_grade_if_interesting(user, course_run_key, mode, status, letter_grade, status = cert.status except GeneratedCertificate.DoesNotExist: # We only care about grades for which there is a certificate. + if verbose: + log.info( + "Skipping send grade: no cert for user [{username}] & course_id [{course_id}]".format( + username=getattr(user, 'username', None), + course_id=str(course_run_key) + ) + ) return # Don't worry about whether it's available as well as awarded. Just awarded is good enough to record a verified # attempt at a course. We want even the grades that didn't pass the class because Credentials wants to know about # those too. if mode not in INTERESTING_MODES or status not in INTERESTING_STATUSES: + if verbose: + log.info( + "Skipping send grade: mode/status uninteresting for mode [{mode}] & status [{status}]".format( + mode=mode, + status=status + ) + ) return # If the course isn't in any program, don't bother telling Credentials about it. When Credentials grows support # for course records as well as program records, we'll need to open this up. if not is_course_run_in_a_program(course_run_key): + if verbose: + log.info( + "Skipping send grade: course run not in a program. [{course_id}]".format(course_id=str(course_run_key)) + ) return # Grab grades if we don't have them in hand if letter_grade is None or percent_grade is None: grade = CourseGradeFactory().read(user, course_key=course_run_key, create_if_needed=False) if grade is None: + if verbose: + log.info( + "Skipping send grade: No grade found for user [{username}] & course_id [{course_id}]".format( + username=getattr(user, 'username', None), + course_id=str(course_run_key) + ) + ) return letter_grade = grade.letter_grade percent_grade = grade.percent @@ -85,15 +137,23 @@ def send_grade_if_interesting(user, course_run_key, mode, status, letter_grade, send_grade_to_credentials.delay(user.username, str(course_run_key), True, letter_grade, percent_grade) -def handle_grade_change(user, course_grade, course_key, **_kwargs): +def handle_grade_change(user, course_grade, course_key, **kwargs): """ Notifies the Credentials IDA about certain grades it needs for its records, when a grade changes. """ - send_grade_if_interesting(user, course_key, None, None, course_grade.letter_grade, course_grade.percent) - - -def handle_cert_change(user, course_key, mode, status, **_kwargs): + send_grade_if_interesting( + user, + course_key, + None, + None, + course_grade.letter_grade, + course_grade.percent, + verbose=kwargs.get('verbose', False) + ) + + +def handle_cert_change(user, course_key, mode, status, **kwargs): """ Notifies the Credentials IDA about certain grades it needs for its records, when a cert changes. """ - send_grade_if_interesting(user, course_key, mode, status, None, None) + send_grade_if_interesting(user, course_key, mode, status, None, None, verbose=kwargs.get('verbose', False)) diff --git a/openedx/core/djangoapps/programs/signals.py b/openedx/core/djangoapps/programs/signals.py index 65d8a3749ec..f64d10b39e9 100644 --- a/openedx/core/djangoapps/programs/signals.py +++ b/openedx/core/djangoapps/programs/signals.py @@ -81,13 +81,41 @@ def handle_course_cert_changed(sender, user, course_key, mode, status, **kwargs) # the credentials app is loaded, resulting in a Django deprecation warning. from openedx.core.djangoapps.credentials.models import CredentialsApiConfig + verbose = kwargs.get('verbose', False) + if verbose: + msg = "Starting handle_course_cert_changed with params: "\ + "sender [{sender}], "\ + "user [{username}], "\ + "course_key [{course_key}], "\ + "mode [{mode}], "\ + "status [{status}], "\ + "kwargs [{kw}]"\ + .format( + sender=sender, + username=getattr(user, 'username', None), + course_key=str(course_key), + mode=mode, + status=status, + kw=kwargs + ) + + LOGGER.info(msg) + # Avoid scheduling new tasks if certification is disabled. if not CredentialsApiConfig.current().is_learner_issuance_enabled: + if verbose: + LOGGER.info("Skipping send cert: is_learner_issuance_enabled False") return # Avoid scheduling new tasks if learner records are disabled for this site (right now, course certs are only # used for learner records -- when that changes, we can remove this bit and always send course certs). if not helpers.get_value_for_org(course_key.org, 'ENABLE_LEARNER_RECORDS', True): + if verbose: + LOGGER.info( + "Skipping send cert: ENABLE_LEARNER_RECORDS False for org [{org}]".format( + org=course_key.org + ) + ) return # schedule background task to process -- GitLab