Skip to content

SE-3149 Fix the theme compilation issue in juniper - #24972

Closed
pkulkark wants to merge 1 commit into
openedx:open-release/juniper.masterfrom
open-craft:pooja/juniper-fix-theme-compilation
Closed

SE-3149 Fix the theme compilation issue in juniper#24972
pkulkark wants to merge 1 commit into
openedx:open-release/juniper.masterfrom
open-craft:pooja/juniper-fix-theme-compilation

Conversation

@pkulkark

@pkulkark pkulkark commented Sep 12, 2020

Copy link
Copy Markdown
Contributor

CachedStaticFilesStorage is deprecated in Django 2.2. It's plugin model i.e. CachedFilesMixin was being used in theme asset compilation. This was leading to inconsistent theme asset compilation and would lead to failures if more than one instance is used for load-balancing. This PR replaces the deprecated CachedFilesMixin with ManifestFilesMixin. It also fixes the order of the modules used in compilation.

Jira Tickets: OSPR-4966

Dependecies: None

Sandbox URL: TBD - sandbox is being provisioned

Merge deadline: None

Testing Instructions:

  1. Spawn a new edX juniper instance and check the css files generated in /edx/var/edxapp/staticfiles/css.
  2. Run paver update_assets command and recheck that no new css files are generated in /edx/var/edxapp/staticfiles/css.
  3. Spawn multiple instances and add to load-balancer. Verify that the assets are getting loaded correctly.

Reviewers:

@openedx-webhooks openedx-webhooks added needs triage open-source-contribution PR author is not from Axim or 2U labels Sep 12, 2020
@openedx-webhooks

Copy link
Copy Markdown

Thanks for the pull request, @pkulkark! I've created OSPR-4966 to keep track of it in JIRA, where we prioritize reviews. Please note that it may take us up to several weeks or months to complete a review and merge your PR.

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

  • supporting documentation
  • Open edX discussion forum 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 be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

@Kelketek

Copy link
Copy Markdown
Contributor

jenkins run all

@Kelketek

Copy link
Copy Markdown
Contributor

👍

  • I tested this: Verified these changes produce consistent asset file compilation.
  • I read through the code

@natabene

Copy link
Copy Markdown
Contributor

@pkulkark Thank you for your contribution. Please let me know once this is ready for our review.

@openedx-webhooks openedx-webhooks added waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. and removed needs triage labels Sep 14, 2020
@pkulkark

Copy link
Copy Markdown
Contributor Author

@natabene I can't seem to get the jenkins tests running. Could you trigger it for me? Thanks!

@natabene

Copy link
Copy Markdown
Contributor

jenkins run all

@nedbat

nedbat commented Sep 14, 2020

Copy link
Copy Markdown
Contributor

Does this correspond to a change on master? Does master need the same fix?

@pkulkark

Copy link
Copy Markdown
Contributor Author

@nedbat Yes it looks like master will need the same fix. This would need to be present since the juniper release so should I create another PR against master?
@natabene I see the message "There are no nodes with the label ‘juniper-worker’" in the console of the jenkins jobs. Is there something I need to do about it?

@nedbat

nedbat commented Sep 15, 2020

Copy link
Copy Markdown
Contributor

@pkulkark Generally we prefer fixes to go on master first, and then be cherry-picked onto the release branch. This helps confirm the fix.
We're still working on getting the Juniper test workers going, there's nothing you need to do.

@pkulkark
pkulkark changed the base branch from open-release/juniper.master to master September 16, 2020 06:45
@pkulkark
pkulkark changed the base branch from master to open-release/juniper.master September 16, 2020 06:46
@pkulkark

Copy link
Copy Markdown
Contributor Author

@nedbat I see. I opened #24990 against master. Closing this one in favor of #24990.

@pkulkark pkulkark closed this Sep 16, 2020
@openedx-webhooks openedx-webhooks added rejected and removed waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. labels Sep 16, 2020
@openedx-webhooks

Copy link
Copy Markdown

@pkulkark Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future.

@edx-status-bot

Copy link
Copy Markdown

Your PR has finished running tests. The following contexts failed:

  • jenkins/python-3.8/a11y
  • jenkins/python-3.8/js
  • jenkins/juniper/a11y
  • jenkins/juniper/js
  • jenkins/juniper/quality
  • jenkins/juniper/python
  • jenkins/python-3.8/python
  • jenkins/python
  • jenkins/quality
  • jenkins/python-3.8/quality

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

open-source-contribution PR author is not from Axim or 2U rejected

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants