Support externally hosted comprehensive themes

[fghaas]

The old Stanford-style themes can be pulled in from an external Git repo; no such provision was previously available for comprehensive themes.

• Introduce two new configuration variables:
  • EDXAPP_COMPREHENSIVE_THEME_SOURCE_REPO: Git repo for the comprehensive theme
  • EDXAPP_COMPREHENSIVE_THEME_VERSION: Git branch, tag, or revision to check out from the repo
• Introduce a new task, "checkout comprehensive theme", doing the equivalent of the previously existing "checkout theme" task. Check out the theme repo to EDXAPP_COMPREHENSIVE_THEME_DIR. If
  EDXAPP_COMPREHENSIVE_THEME_SOURCE_REPO is unset, continue to assume that EDXAPP_COMPREHENSIVE_THEME_DIR already exists and is populated with a comprehensive theme.
• Rename the "checkout theme" task to "checkout Stanford-style theme".

[openedx-webhooks]

Thanks for the pull request, @fghaas! I've created OSPR-1063 to keep track of it in JIRA. JIRA is a place for product owners to prioritize feature reviews by the engineering development teams.

Feel free to add as much of the following information to the ticket:

• supporting documentation
• edx-code email threads
• timeline information ("this must be merged by XX date", and why that is)
• partner information ("this is a course on edx.org")
• any other information that can help Product understand the context for the PR

All technical communication about the code itself will still be done via the GitHub pull request interface. As a reminder, our process documentation is here.

[nedbat]

@singingwolfboy will have opinions on this.

[singingwolfboy]

I am way, way in favor of the idea behind this change. We would love to move the edx.org and edge.edx.org themes out of the edx-platform repository, and into their own repositories. Similarly, others should be able to have themes that are separate repositories, particularly because a theme will have lots of trademarked content like logos, and the website operator will probably want to keep that content private.

I can't provide a thumbs-up or thumbs-down on this pull request, since I'm not familiar with the configuration repo, and I don't understand how this code works. But conceptually, this is a very, very good idea.

[mattdrayer]

@ziafazal, FYI

[mattdrayer]

@fghaas I'm with @singingwolfboy in that I don't have a solid understanding of the configuration aspect of the platform works. That said, I'm wondering if it's possible to change the name here to something more generic, versus "Stanford-style"?

[fghaas]

@mattdrayer The only authoritative resource on comprehensive theming that I've been able to find is https://github.com/edx/edx-platform/blob/master/themes/README.rst, and it refers to the opposite of comprehensive themes as "Stanford" theming (https://github.com/edx/edx-platform/blob/master/themes/README.rst#stanford-theming). Confusing as theming already is, I'd prefer not to invent a new term for the sole purpose of being more generic.

[singingwolfboy]

@fghaas there's more documentation for comprehensive theming available here: http://edx.readthedocs.org/projects/edx-installing-configuring-and-running/en/latest/theming/index.html

[fghaas]

@singingwolfboy Good to know, thanks.

[mattdrayer]

@fghaas -- Okay, I've read thru all of the docs and I now agree that sticking with "Stanford-style" makes sense here. Can you please include a comment with the URL pointing to the related section of the README.rst and then this changeset should be good to go.

[fghaas]

@mattdrayer You mean a comment right there in the playbook YAML? Or somewhere else?

[mattdrayer]

Yep, I'd put it right there in the YAML file, just so people know where to look for more info when they come across that section.

[fghaas]

@mattdrayer Done.

[mattdrayer]

Nice, this looks great to me now -- thanks very much @fghaas! 👍

[fghaas]

Thanks @mattdrayer, do you still need to remove or add some tags so this gets merged?

[mattdrayer]

@fghaas I'm not sure who is responsible for merging changes to edx/configuration (maybe it's me!) -- @feanil or @fredsmith do you have any advice to lend?

[fredsmith]

@fghaas - the fact that you're re-using edxapp_git_ssh means that you'll be reusing the credentials that you use for checking out edxapp. eg. if you have a private theme and a public edxapp, this won't work. You should set this as a separate variable that defaults to edxapp_git_ssh.

Also, I think the EDXAPP_COMPREHENSIVE_THEME_DIR should have a more useful default, probably based on {{ edxapp_app_dir }} @edx/devops any other comments?

[fghaas]

@fredsmith, re edxapp_git_ssh you'll notice that the pre-existing task for checking out a Stanford theme does exactly the same thing. Are you saying that's broken too?

[fghaas]

@fredsmith Just letting you know that I'm still waiting for an answer here.

[fredsmith]

@fghaas yes, the existing play is also broken in this way. I suppose if it's not an issue for you, and it's not an issue for us, we can wait until it's an issue for someone before we implement the additional complexity of more data structures. @feanil thoughts?

[stvstnfrd]

@fghaas @fredsmith Re: The playbook for Stanford Theme

FWIW: We use this play successfully with a public platform and private theme, setting values:

EDXAPP_USE_GIT_IDENTITY: true
EDXAPP_GIT_IDENTITY: WOULDNT_YOU_LIKE_TO_KNOW

[jbarciauskas]

@fredsmith @feanil what's the next step here?

[feanil]

@jbarciauskas I'm hoping to do a review of this today or tomorrow but I'm on call this week so my schedule is a bit unpredictable.

[jbarciauskas]

Understood, thanks!

[fghaas]

Now that Dogwood is out and comprehensive theming is touted as a key feature, could this perhaps be reviewed? It's only been sitting here for a month and a half.

[feanil]

👍 @fghaas sorry about that, I dropped the ball on this. It got buried in my inbox.

[fghaas]

Thanks @feanil, would be great to see this merged.

[feanil]

I'll be merging it once the tests have finished running.

[fghaas]

@feanil Thank you!