Conversation
cms/djangoapps/contentstore/tasks.py
Outdated
| return | ||
|
|
||
| if not user_has_access(user): | ||
| logging.info(f'Course import {course_key_string}: User is not allowed to import') |
cms/djangoapps/contentstore/tasks.py
Outdated
| return | ||
|
|
||
| if not file_is_supported(): | ||
| logging.info(f'Course import {course_key_string}: File not supported') |
cms/djangoapps/contentstore/tasks.py
Outdated
| return olx_is_valid | ||
| try: | ||
| if str(courselike_key) == "course-v1:ArbiX+CS101+2014_T3": | ||
| logging.info(log_prefix + "Validating. Calling olxcleaner.validate") |
There was a problem hiding this comment.
This is the default prefix they are using in every log of this function. This string is equal to f"Course import {courselike_key}"
cms/djangoapps/contentstore/tasks.py
Outdated
| if str(courselike_key) == "course-v1:ArbiX+CS101+2014_T3": | ||
| logging.info(log_prefix + "Course validated. No errors") | ||
| except Exception as e: # pylint: disable=broad-except | ||
| LOGGER.exception(f'{log_prefix}: CourseOlx could not be validated' + str(e)) |
There was a problem hiding this comment.
LOGGER.exception automatically prints the stack trace so I don't think `str(e) is needed here.
common/djangoapps/util/monitoring.py
Outdated
| """ | ||
| set_custom_attribute('course_import_failure', import_step) | ||
| set_custom_attributes_for_course_key(course_key) | ||
| logging.info(f"Course import failed at step {import_step} for course with key {course_key}") |
There was a problem hiding this comment.
shouldn't these also be behind the course key condition?
…/xmodule/xmodule/modulestore in files xml.py and xml_importer.py
saadyousafarbi
left a comment
saadyousafarbi
left a comment
There was a problem hiding this comment.
Can you please add a prefix of Investigation Log: to all the conditional logs we are adding, so other developers have an idea of why these logs have been added!
| allowed_xblocks=ALL_ALLOWED_XBLOCKS | ||
| ) | ||
| except Exception: # pylint: disable=broad-except | ||
| except Exception as e: # pylint: disable=broad-except |
There was a problem hiding this comment.
I added "as e" after Exception to print it. Awais bhai told me LOGGER.Exception prints exception. Restored old code.
| if str(target_course_id) == "course-v1:ArbiX+CS101+2014_T3": | ||
| logging.info(f"Investigation Log: {target_course_id} : Course Descriptor is None") |
There was a problem hiding this comment.
this shouldn't be part of the if course_descriptor is None: block. the logs should be immediately after the finally: block.
logging.info(f"Investigation Log: {target_course_id} : {course_descriptor} ") something like this.
| # If we fail to load the course, then skip the rest of the loading steps | ||
| if isinstance(course_descriptor, ErrorBlock): | ||
| if str(target_course_id) == "course-v1:ArbiX+CS101+2014_T3": | ||
| logging.info(f"Investigation Log: {target_course_id} : Course Descriptor is instance of ErrorBlock") |
There was a problem hiding this comment.
I see single quotes here in this file, please use single quotes here too.
3 participants
Added logs near course validation in course import.