Skip to content
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

[#10928] Add script to list active instructors within a period #10947

Conversation

daongochieu2810
Copy link
Contributor

Fix #10928
Solution outline:

  • Filter feedback sessions based on given start and end times
  • Get creator email from filtered sessions and print them

@madanalogy madanalogy added the s.ToReview The PR is waiting for review(s) label Feb 7, 2021
Copy link
Contributor

@Derek-Hardy Derek-Hardy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@damithc
Copy link
Contributor

damithc commented Feb 10, 2021

I gave it a try. Took 22 minutes to finish, and produced 1.6k results. So, the approach is not very scalable. It might even time out when we have even more sessions. We have 66K sessions by the way.
If the sessions are given to us in the order they were created, perhaps we can terminate after we hit a session that's outside of the period?

@daongochieu2810
Copy link
Contributor Author

I gave it a try. Took 22 minutes to finish, and produced 1.6k results. So, the approach is not very scalable. It might even time out when we have even more sessions. We have 66K sessions by the way.
If the sessions are given to us in the order they were created, perhaps we can terminate after we hit a session that's outside of the period?

got it, I will try to prune it as you suggested

@rrtheonlyone
Copy link
Contributor

You do not need to binary search after fetching ALL sessions.

I believe you can do this with just one query that can filter by created time - like this (just an arbitrary example):

ofy().load().type(CourseStudent.class)
                   .filter("createdAt >", queryEntitiesFrom)
                   .filter("createdAt <=", queryEntitiesTo)

Furthermore, since the creatorEmail is indexed, you can then do a projection on the query to retrieve it - project().

It will look something like - ofy.load().type(..).filter(..).filter(..).project(..)

This will allow you to do just one query to fetch everything you need. This should be much faster as the database will help optimize for you.

Copy link
Contributor

@rrtheonlyone rrtheonlyone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor Change - do test the script locally before pushing in future :)

Copy link
Contributor

@rrtheonlyone rrtheonlyone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good - prof @damithc do see if this is faster when you have time.

This script finds the list of active instructors using the start time of the feedback sessions they have created. This is just a heuristic - I am not sure if we can find all active instructors using this.

@damithc
Copy link
Contributor

damithc commented Feb 14, 2021

Looks good - prof @damithc do see if this is faster when you have time.

This one takes about 4 minutes. Nice!

@madanalogy madanalogy removed the request for review from damithc February 15, 2021 02:08
@madanalogy madanalogy assigned rrtheonlyone and unassigned damithc Feb 15, 2021
@madanalogy madanalogy added s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging c.Feature User-facing feature; can be new feature or enhancement to existing feature and removed s.ToReview The PR is waiting for review(s) labels Feb 15, 2021
@madanalogy madanalogy added this to the V7.10.0 milestone Feb 15, 2021
@madanalogy madanalogy merged commit 05562cc into TEAMMATES:master Feb 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c.Feature User-facing feature; can be new feature or enhancement to existing feature s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a script for the admin to generate a list of active instructors
5 participants