Vary all pegasus pages on language and user_type #14078
Conversation
Enables future work for all cached pages to vary on language and user_type in the page header.
| # Language drop-down selection. | ||
| 'language_', | ||
| # Page mode, for A/B experiments and feature-flag rollouts. | ||
| 'pm', |
There was a problem hiding this comment.
For what it's worth I won't be using this yet, because it's not per-user (right?), in case you want to reduce cache variations. But no problem if you're happy to put it in.
There was a problem hiding this comment.
this PR doesn't change anything related to the use of this pm cookie (other than adding an explanatory comment), so the behavior of this cookie should not change at all here.
To answer/clarify your question about whether pm is 'per-user':
- No, the
pmcookie value does not (and should not) contain a unique value for each individual user. (Using unique values in a cache-key makes the cached pages nearly useless, so I wouldn't recommend it.) - Yes, you could write application code to set/modify the
pmcookie value for individual user-agent/s, for example, by settingpmcookie to equaltest_headerfor a few select test users on sign-in, so that only those specific users would see a specialtest_headerpage variant.
There was a problem hiding this comment.
Ah, of course. We can definitely use this by having a few internal users manually set a pm cookie so that they see the new header work before anybody else. Thanks.
|
|
||
| DEFAULT_COOKIES = [ | ||
| # Language drop-down selection. | ||
| 'language_', |
There was a problem hiding this comment.
I'm trying to think whether there are any implications, particularly on pegasus, for pages that aren't currently localized. It's possible that seeing a non-en language cookie could cause them to display the wrong thing. @tanyaparker do you happen to remember offhand?
There was a problem hiding this comment.
We had some caching issues before when we were on a page that wasn't supposed to be localized but contained a view that was localized. Example of a page that contains the share_buttons view that is localized. Or code.org/volunteer where we started to localize the fields but never enabled localization on the page. And we had caught different languages popping up.
Is this what you mean? Will fixed those issues.
There was a problem hiding this comment.
Yeah, the behavior on pages like the ones Tanya mentioned could actually change as a result of this PR. This will effectively 'enable localization' on all of Pegasus pages by default (so 'enabling localization' for a page will no longer be necessary), so any pages that are (partially or completely) localized will start to show different content for different languages. If enabling localization could cause problems for any page, we might want to do an i18n pass on 'never enabled localization' pegasus pages after this gets merged to make sure there are no issues.
There was a problem hiding this comment.
Note that this is different from the kind of behavior we saw with old language-mixing bugs, where a single cached page would get 'poisoned' by a mix of differently-localized renderings. That was caused by the language header/cookie not being present in the cache key, and we fixed this by actively removing headers/cookies and replacing them with a default (in our case en-US for the language header) whenever they're not present in the cache key.
With this PR the language header/cookie is always present in the cache key, so all pages will be served by separate language-specific caches.
| headers: [], | ||
| cookies: 'none' | ||
| headers: LANGUAGE_HEADER, | ||
| cookies: DEFAULT_COOKIES |
There was a problem hiding this comment.
This diff changes default caching behavior for all Pegasus pages so that they will be cached separately per-language by default. Since all pages will now be cache-localized by default, we no longer need to special-case separate behaviors for localized paths (/learn*, /congrats removed above).
There was a problem hiding this comment.
Does this mean all pages are automatically localizable? So this means we may get pages with mixed EN/non-EN...
There was a problem hiding this comment.
Saw your other comment. There are only a few cases (/volunteer, /applab) where we have some I18n.t mixed with English. For /volunteer I think it's been up long enough that translations should be available for many languages so it's a benefit to turn localization on for this page. For /applab and any other cases I'll sort out whether to make them all English or all localized. Either way, I think this is a good change and these pages being in a mixed state shouldn't block merging in this PR.
This PR changes the HTTP cache configuration to vary all server-side html responses based on the value of the
user_typecookie implemented in #14077 to reflect the logged-in and teacher/student status of the browser.Note that this will increase the total number of Pegasus page requests hitting our frontend servers by a factor of ~3 for existing localized pages (and a factor of [number of languages * 3] for all other pages not previously localized). This shouldn't be a problem overall, but it's worth monitoring this rollout for any potential performance impact just in case.