Hundreds of 404 errors for string files in Studio. #1308
Comments
|
In Slack, @samreid said:
|
|
That would be the plan, and it would presumably need a rosetta change (and seems a bit fragile). Thoughts on whether that would be worth avoiding the 404s? Alternatively, couldn't we just... create a bunch of empty files, one for every locale/repo? @samreid thoughts on this? |
|
I think avoiding the 404 errors is important. They could result in someone missing important errors. I'm certainly not going to scroll back to the beginning of that console output to see if any important errors preceeded the 404 errors. |
|
I don't understand why creating empty files that Rosetta presumably wouldn't be fetching would negatively affect Rosetta. I'd like to understand this further, but I'm not on PhET hours right now. |
|
@zepumph also reported that loading some 1500 404s on the state wrapper was causing timeout errors, we speculated that loading that many non-404s may have the same problem? But we would have to solve that to support 100% i18n anyways, so maybe we should start there? |
|
Can we discuss this in dev meeting? |
|
I wrote this script to test creating all the locale files: // Copyright 2022, University of Colorado Boulder
// Copied from load-unbuilt-strings.js
const locales = 'aa,ab,ae,af,ak,am,an,ar,ar_MA,ar_SA,as,av,ay,az,ba,be,bg,bh,bi,bm,bn,bo,br,bs,ca,ce,ch,co,cr,cs,cu,cv,cy,da,de,dv,dz,ee,el,en,en_CA,en_GB,eo,es,es_CO,es_CR,es_ES,es_MX,es_PE,et,eu,fa,ff,fi,fj,fo,fr,fu,fy,ga,gd,gl,gn,gu,gv,ha,hi,ho,hr,ht,hu,hy,hz,ia,ie,ig,ii,ik,in,io,is,it,iu,iw,ja,ji,jv,ka,kg,ki,kj,kk,kl,km,kn,ko,kr,ks,ku,ku_TR,kv,kw,ky,la,lb,lg,li,lk,ln,lo,lt,lu,lv,mg,mh,mi,mk,ml,mn,mo,mr,ms,mt,my,na,nb,nd,ne,ng,nl,nn,nr,nv,ny,oc,oj,om,or,os,pa,pi,pl,ps,pt,pt_BR,qu,rm,rn,ro,ru,rw,ry,sa,sc,sd,se,sg,sh,si,sk,sl,sm,sn,so,sq,sr,ss,st,su,sv,sw,ta,te,tg,th,ti,tk,tl,tn,to,tr,ts,tt,tw,ty,ug,uk,ur,uz,ve,vi,vo,wa,wo,xh,yo,za,zh_CN,zh_HK,zh_TW,zu'.split( ',' );
// Find all directories in a directory
const fs = require( 'fs' );
const dirs = fs.readdirSync( '../babel', { withFileTypes: true } )
.filter( dirent => dirent.isDirectory() )
.map( dirent => dirent.name ).filter( dir => dir !== '.git' );
dirs.push( 'sun' );
dirs.push( 'tambo' );
dirs.push( 'twixt' );
dirs.forEach( dir => {
locales.forEach( locale => {
if ( locale !== 'en' ) {
const repo = dir;
const filename = `../babel/${repo}/${repo}-strings_${locale}.json`;
if ( !fs.existsSync( filename ) ) {
try {
fs.mkdirSync( `../babel/${repo}` );
}
catch( e ) {}
fs.writeFileSync( filename, `{
}` );
}
}
} );
} );Running it, I observed the following:
|
|
My network panel for GAO with the strings:
and reverting to the 404s:
So maybe the 200s are a little faster than the 404s? |
and unbuilt mode:
This will make testing more difficult. For instance, @KatieWoe discovered a layout bug in phetsims/qa#830 but I couldn't reproduce it locally due to this problem. |
|
This problem is making it take 35 seconds to launch one sim on phettest:
Also discovered by @Nancy-Salpepi in phetsims/gravity-and-orbits#434 (comment). Not sure if changing 404s to 200s would fix it or not. It may be that requesting too many locales would be too slow anyways. |
|
From today's dev meeting: @kathy-phet thinks creating these files eagerly is problematic. @jonathanolson says: When rosetta makes a change. We just bundle them into one conglomerate string file per that has everything for that repo. @jonathanolson one file per repo in babel that has just the strings (no history). Like joist-strings.json with all the locales. Then when loading a sim like friction, it wouldn't include CCK strings. @zepumph asks: A script that runs a postprocess step, runs once a night. Doesn't need to hook into rosetta. Just a development tool. |
|
@liammulh and I discussed this in our collaboration meeting today. We're in agreement that having a dummy locale file for every possible locale would be problematic, and I think it would get way worse if and when we expand the list of languages we use, which is probably inevitable, since we've already talked about moving from ISO 639-1 to ISO 639-3. We came up with an alternative approach to the idea of creating a conglomerated file with all the strings: Create just a list of all locales for which a translation exists in each repo. Then the
Example of what the string list file would look like:
{
"locales": [ "ar", "ar_SA", "de", ... ]
}Assigning back to @jonathanolson and @samreid to get their input. |
|
A locales file sounds reasonable to me and preferable to a conglomerate file. But I wonder if we can skip this step by relying on everyone's http-server to output a file listing? For instance, if I navigate to: http://localhost/main/babel/gravity-and-orbits/, I see
Would it be easy enough to xhr that page, and use a regex to detect locales? I'm using http-server, but also saw similar output with apache. This way it would always be up-to-date and we wouldn't need any scripts or management or maintenance for it. |
That would only work if the local copy of babel is current. But I think it would be reasonable if every developer keeps their copy of babel current. One of the reasons JB and I liked the idea of having a |
|
@liammulh and I discussed that it "may" work to use the directory listing, but if even one phet developer or server machine doesn't output a compatible directory listing, then we would need the JSON file. I tested loading https://bayes.colorado.edu/continuous-testing/ct-snapshots/1661987918125/babel from a recent snapshot and was met with this error:
So I think the "directory listing" approach is doomed to fail and we should use the locale list. But now that we are locked in to having to create a file, why not experiment with the conglomerate version? If it saves us even 0.5 seconds every time we launch a sim locally it will be well worth it. |
|
Summarizing recommended work from above:
|
There are actually several repos with no translations. For instance "twixt". This process creates an empty conglomerate file which can be loaded alongside the files that do have translations. I will document it as such. |
|
I think I have addressed or commented on all the recommendations, @jbphet can you please review? |
|
Mostly the responses seem fine. Two questions. First, this is what fallbackLocalesProperty looks like in Studio. The documentation doesn't seem to match what I'm seeing. This might be because I'm in the unbuilt mode, and feel free to let me know if so, but the docs say, "The list MUST end in 'en'", but it doesn't - it's an empty list. This seems potentially confusing to users.
Second, there is a checkmark next to the item above that says, "the task should include an npm update", but I don't see that change in the code and when I run |
|
For the npm install, I added that to the documentation in this commit: ca7595f. Is that sufficient? I don't think our grunt tasks should typically run npm operations (aside from perennial maintenance tasks). Regarding fallbackLocalesProperty, I agree it looks problematic. @jonathanolson can you please comment on that aspect? const fallbackLocalesProperty = new Property<string[]>( [], {
tandem: Tandem.GENERAL_MODEL.createTandem( 'fallbackLocalesProperty' ),
phetioDocumentation: 'an ordered list of locales to fall back on, for example: ["es", "de", "en"]. The list MUST end in "en".says it must end in |
|
@samreid said:
That's fine by me, thanks for the clarification. It makes sense to me that we shouldn't actively run npm install, since that could end up chewing up a lot of disk space. Unassigning myself, leaving it assigned to @jonathanolson to respond to the question above. |
|
fallbackLocales definitely should NOT require containing the fallback locale. It can be empty and works great when empty. I suggest correcting the documentation added in phetsims/joist@9a80165. |
|
|
All is ready here, and final behavior of the fallbackLocalesProperty will be solidified in https://github.com/phetsims/phet-io/issues/1893. Thanks all! |
Related to #1302 ... There are hundreds of 404 errors when attempting to load any sim into Studio.
To reproduce, go to phetmarks, choose any PhET-iO-instrumented sim, choose mode "studio", press "Launch" button.
For example:
The text was updated successfully, but these errors were encountered: