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

Collect jsi18n files via staticfiles to get hashed filenames. #2792

Merged
merged 2 commits into from
Feb 25, 2016

Conversation

Osmose
Copy link
Contributor

@Osmose Osmose commented Feb 23, 2016

STATICI18N_ROOT causes the compilejsi18n command to write the jsi18n files to /jsi18n in the repo root. Then, adding that to STATICFILES_DIRS causes them to be collected during collectstatic, which saves them with the hashes we expect in their filenames.

I had to update the deploy script to collect the jsi18n files before running collectstatic, as well.

@Osmose
Copy link
Contributor Author

Osmose commented Feb 24, 2016

I deployed this on staging and JavaScript localization seems to be working: https://support.allizom.org/af/kpi/dashboard

Because this updates the deploy script itself, it might break on the first push; I'm not entirely sure if the update script will pick up deploy script changes immediately. Break here is defined as an exception on every page that uses the base templates loading this file, which is bad. The issue is that the static helper fails when it can't find a static file that has been requested.

A second deploy should fix the issue. This might be acceptable. If it's not, then I think we we can do is add an extra commit that keeps the deploy and collection changes, but removes the change to generating the URLs in the template. Then, add another commit reverting back to the new URL generation via the static helper. That way, we can deploy only the collection changes but avoid calling the static helper. Then we deploy the latest commit, and things should work!

I can add those changes if we want them. Do we want them?

@mythmon
Copy link
Contributor

mythmon commented Feb 24, 2016

Changes to the deploy script definitely require two pushes. We've run into this before. That would mean about 10 minutes of effective downtime, which I'm not thrilled about.

It sounds like it is pretty easy to split this into two commits that we can deploy in two steps. Lets do that.

@Osmose
Copy link
Contributor Author

Osmose commented Feb 24, 2016

Alternate fix: Do not blow up when generating static URLs that don't exist.

I have implemented that.

@mythmon
Copy link
Contributor

mythmon commented Feb 24, 2016

This works for me locally. Are you happy with your testing on stage/dev? Is this ready to go out?

@Osmose
Copy link
Contributor Author

Osmose commented Feb 24, 2016

I'd like to test it on stage one more time after merging before we hit prod, but I'm otherwise happy with this.

@mythmon
Copy link
Contributor

mythmon commented Feb 25, 2016

Ok. The code here is good. Merge when you're happy with the deployment situation.

r+

Osmose added a commit that referenced this pull request Feb 25, 2016
Collect jsi18n files via staticfiles to get hashed filenames.
@Osmose Osmose merged commit 1cc83cc into mozilla:master Feb 25, 2016
@Osmose Osmose deleted the js-domain branch February 25, 2016 00:23
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