-
Notifications
You must be signed in to change notification settings - Fork 4
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 Status Audit Management Command #667
base: master
Are you sure you want to change the base?
Conversation
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.
lgtm, mostly just nits
) | ||
|
||
# Write out statistics and missing courses to an output file. | ||
with open("./status_audit.txt", "w") as f: |
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.
if we want to run this on a cron, we may want to allow the file name to be passed in/to include the UNIX timestamp.
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.
Yup sounds good. Do you think we should write it to S3 or something like that? Maybe it'd be cool if we could setup a Slack integration and get notifs that way, but might be a lot of extra work for little reward.
|
||
f.write( | ||
"""Courses Out of Sync\nCourse Code / Last Update Status / | ||
Our Stored Status / Actual Status\n""" |
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.
What does actual status mean? Might be good to elaborate bc I'm getting it mixed up in my head
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.
Actual status is the Path@Penn (or the accurate), status of a course. I'll rename it a bit and also add a comment explaining in-line.
stats["missing_data"] += 1 | ||
continue | ||
|
||
course_status = data.get("status") |
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.
is this status per section or per course?
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.
It should be section_status
, will change.
Adding a management command that generates a text file, displaying the differences in section statuses between our database and the OpenData API endpoint
/v1/course_section_status/{term}/all
.