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

User stages #23022

Merged
merged 4 commits into from Jul 9, 2018
Merged

User stages #23022

merged 4 commits into from Jul 9, 2018

Conversation

bencodeorg
Copy link
Contributor

This creates a analytics/Redshift-only table, user_stages, which aggregates student progress to the "stage" level (also known as a "lesson" externally in some of our courses). Our production data model includes student progress at the "level" granularity (user_levels) and the "script" granularity (user_scripts), but doesn't actively track anything in between.

This PR:

  • creates a redshift_rollups cron task to run every playbook in the aws/redshift/playbooks directory, and adds a new playbook to create/refresh user_stages.

@bencodeorg bencodeorg requested a review from wjordan June 11, 2018 22:12
@bencodeorg
Copy link
Contributor Author

bencodeorg commented Jun 11, 2018

Ah I thought the [ci skip] tag on the second commit would just prevent a circle build for the second commit (which is a SQL change with no production implications) instead of for the whole PR. The first modifies a cron job, so would actually be run via a production machine, but I think you've told me before @wjordan there's not really any testing that would be helpful there anyway?

@wjordan
Copy link
Contributor

wjordan commented Jun 12, 2018

Is there a specific reason favoring adding a second playbook, versus adding an extra step to the existing playbook?

Copy link
Contributor

@wjordan wjordan left a comment

Choose a reason for hiding this comment

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

the referenced tables/user_stages.sql seems to be missing

@bencodeorg
Copy link
Contributor Author

D'oh. Added user_stages script.

Re: adding second playbook, there's no functional difference, just an organizational change. All the stuff in the first playbook relates explicitly to reporting we do to regional partners, and the new one for user stages is more broadly useful for analytical purposes.

My thought was that we might add additional playbooks in the future as we do more of this sort of stuff (I'd more descriptively call these "analytical aggregates"), and was trying to prevent a single playbook from getting unwieldy.

@bencodeorg
Copy link
Contributor Author

@wjordan this look ok to you now?

Copy link
Contributor

@wjordan wjordan left a comment

Choose a reason for hiding this comment

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

👍 looks great, sorry for losing track of this.

@bencodeorg bencodeorg merged commit 2262ea4 into staging Jul 9, 2018
@bencodeorg bencodeorg deleted the user-stages branch July 11, 2018 21:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants