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

WIP: Add a custom theme to edxapp LMS #105

Closed
wants to merge 1 commit into from

Conversation

sampaccoud
Copy link
Contributor

@sampaccoud sampaccoud commented Aug 27, 2018

Purpose

Adding a custom theme to an Open edX site must be done upon deployment and not in the common Docker image because it is specific to each customer.

edit: this PR is depends on #111

Proposal

We propose to use OpenShift's buildconfig objects to automate such builds. The same mechanism can be used to add monitoring or other build steps that are specific either to our way to deploy things to OpenShift or to a customer (like the theme).

It is a good idea to use a common image stream for the CMS and the LMS even if no customization is made because it allows keeping a copy of the image in our local registry and avoid downloading it
from DockerHub upon each deployment.

  • Add a build config step and an image stream
  • Upgrade edxapp docker image version to gingko.1-1.0.7
  • Remove the DEFAULT_SITE_THEME env var (see WIP: Add a custom theme to edxapp LMS #105 (comment))
  • Add a private SSH key as a sshauth secret to connect to the theme as a private repository in Gitlab

The theme is added in a build step specific to each customer upon
deployment. The same build will be used to add functionalities
on the common image that are either specific to our OpenShift
deployment (e.g. adding Datadog monitoring) our specific to
a customer (theme).

It is a good idea to use a common image stream for the CMS and the
LMS even if no customization is made because it allows keeping a
copy of the image in our local registry and avoid downloading it
from Doker Hub upon each deployment.
@sampaccoud sampaccoud mentioned this pull request Aug 28, 2018
3 tasks
@jmaupetit jmaupetit changed the title Add a custom theme to edxapp LMS WIP: Add a custom theme to edxapp LMS Aug 28, 2018
@jmaupetit jmaupetit self-assigned this Aug 28, 2018
USER 0
{% if edxapp_theme_url is defined and edxapp_theme_url %}
COPY . /edx/app/edxapp/edx-platform/themes/custom-theme
NO_PREREQ_INSTALL=1 DEFAULT_SITE_THEME="custom-theme" paver update_assets --settings={{ edxapp_build_settings }} --skip-collect
Copy link
Contributor Author

@sampaccoud sampaccoud Aug 28, 2018

Choose a reason for hiding this comment

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

@jmaupetit in fact, I think it might not be required to set DEFAULT_SITE_THEME since "update_assets" builds all the themes in the themes directory...
Maybe try removing it as it would be simpler... and we wouldn't need to modify the build settings in fun-platform like I did...

jmaupetit added a commit that referenced this pull request Sep 4, 2018
This work adapts @sampaccoud's work on #105 to add customizable themes
for the LMS. We've added an example for patient0/development with a
public theme cooked by the Racoongang team.
@jmaupetit jmaupetit mentioned this pull request Sep 4, 2018
3 tasks
jmaupetit added a commit that referenced this pull request Sep 4, 2018
This work adapts @sampaccoud's work on #105 to add customizable themes
for the LMS. We've added an example for patient0/development with a
public theme cooked by the Racoongang team.
@sampaccoud sampaccoud closed this Sep 4, 2018
@jmaupetit jmaupetit deleted the add-theme-to-edxapp-lms branch September 4, 2018 15:34
jmaupetit added a commit that referenced this pull request Sep 4, 2018
This work adapts @sampaccoud's work on #105 to add customizable themes
for the LMS. We've added an example for patient0/development with a
public theme cooked by the Racoongang team.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants