-
Notifications
You must be signed in to change notification settings - Fork 14
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
Schedule fetching canvas groups rosters #6906
Conversation
e37a1bf
to
77f4759
Compare
@@ -156,6 +158,69 @@ def schedule_fetching_assignment_rosters() -> None: | |||
) | |||
|
|||
|
|||
def schedule_fetching_segment_rosters() -> None: |
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.
This is very similar to the existing assignment and course schedulers.
# Only fetch roster for segments for which we haven't schedule a fetch recently | ||
no_recent_scheduled_roster_fetch_clause = ~exists( | ||
select(TaskDone).where( | ||
TaskDone.key == func.concat("roster::segment::scheduled::", LMSSegment.id), |
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.
Key for segments
.join(Course, Event.course_id == Course.id) | ||
.where( | ||
Event.timestamp >= now - LAUNCHED_WINDOW, | ||
Course.authority_provided_id == LMSCourse.h_authority_provided_id, |
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.
We based this on any launches on the courses the segments belong to.
no_recent_scheduled_roster_fetch_clause, | ||
recent_launches_clause, | ||
# Only canvas groups for now | ||
LMSSegment.type == "canvas_group", |
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.
We are limiting segments to only "canvas_group"s only
@@ -180,9 +245,25 @@ def fetch_course_roster(*, lms_course_id) -> None: | |||
retry_backoff_max=7200, | |||
) | |||
def fetch_assignment_roster(*, assignment_id) -> None: | |||
"""Fetch the roster for one course.""" | |||
"""Fetch the roster for one assignment.""" |
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.
Fix existing comment.
77f4759
to
48080f3
Compare
Use the same logic as for assignments and courses.
48080f3
to
4509b1f
Compare
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
Use the same logic as for assignments and courses.
For:
Testing
https://hypothesis.instructure.com/courses/319/assignments/3336
make shell
, schedule fetching rosterssegment roster fetching is scheduled:
schedule_fetching_rosters.delay()
No segment roster fetch is scheduled now.
Schedule the fetch again, nothing happens yet
Change the date of the existing roster:
Schedule again, this time both rosters are re-fetched
Lets now remove the task done and mark the roster as old: