search

phetsims / perennial

Maintenance tools that won't change with different versions of chipper checked out
http://scenerystack.org/
MIT License
4 stars 5 forks source link

Should translations trigger an update of phet-io files? #53

Open mattpen opened 6 years ago

mattpen commented 6 years ago

We are now creating phet-io sims that have all locales included and accessible via query parameter. Should we automatically update the phet-io sims when translations are submitted?

samreid commented 6 years ago

Unless it is very easy, I'd recommend to wait on adding that step until the future (when there is need for it, and when our processes have stabilized).

ariel-phet commented 6 years ago

@zepumph @samreid @mattpen

My thinking would be that yes, the phet-io sims would get an automatic update, but we would not consider this a new version of the sim in terms of number scheme. Is that thinking incorrect? To me the sim code has not changed at all, we are just adding more strings.

Might be worth talking about at dev meeting as well. We would want the non-io all.html file also automatically updated so that each time a new installer is built it includes latest

ariel-phet commented 6 years ago

I am also fine with punting for now as @samreid suggests

samreid commented 6 years ago

but we would not consider this a new version of the sim in terms of number scheme. Is that thinking incorrect? To me the sim code has not changed at all, we are just adding more strings.

I'm not exactly sure what the process is for the phet-brand sims (perhaps it increments the maintenance number?), but we should use the same process for phet-io brand.

mattpen commented 6 years ago

My thinking would be that yes, the phet-io sims would get an automatic update, but we would not consider this a new version of the sim in terms of number scheme.

Translations will not increment the version number (not even the maintenance number) in either phet or phet-io.

We would want the non-io all.html file also automatically updated so that each time a new installer is built it includes latest

I think this work is already complete (rebuilding the _all file on translation submissions).

@jonathanolson and I decided to open this issue because with the _all files and locale query parameters now being used in phet-io, it would be fairly easy to adapt translation submissions to update production phet-io sims as well.

zepumph commented 6 years ago

Although I agree with @samreid that we are a ways down the road from needing this in phet-io, if it is easy to implement, and we get it for free with phet-io brand for no additional cost, then I don't see why not. Only potential concern I can think of: @mattpen will it just overwrite the old file on the server? Does that mean that the file is not served every time there is a new translation? We don't have varnish (not sure if varnish caches sims) on phet-io so if there were tons of translations, would it mean downtime for a version of the sim?

This brings up another question I have been having. How do you know what translations there are in an all.html? It would be nice to have a way to communicate that to clients.

mattpen commented 6 years ago

will it just overwrite the old file on the server? Does that mean that the file is not served every time there is a new translation?

The file isn't overwritten in place. The build-server writes the new files in a temporary directory then copies them to the public directory. There should effectively be no downtime.

zepumph commented 6 years ago

Cool beans. What about

This brings up another question I have been having. How do you know what translations there are in an all.html? It would be nice to have a way to communicate that to clients.

Is there another (better) issue I could look up to reference/address this?

mattpen commented 6 years ago

@zepumph - I'm not aware of an easy way to parse it from the sim file. You could determine it by parsing the xml file produced with production deploys (e.g. https://phet.colorado.edu/sims/html/circuit-construction-kit-dc/latest/circuit-construction-kit-dc.xml). You could also make a call to the Metadata API, which has a list of localizations for each sim.

If there is a sim in production for phet-io that has not been published for phet these methods will not be reliable. I'm not sure if that ever happens and even if it did it wouldn't be available in rosetta to be translated.

zepumph commented 6 years ago

To bring this back from the dead, here is the way I see the issue:

I think that you should implement this functionality, but maybe keep control of it in a flag that is turned on for now, but if we run into problems with the phet-io sims, then we can turn it off for the builder-server easily so that it doesn't update. @mattpen does this make sense?

zepumph commented 6 years ago

Also I would love to hear from @samreid to see if he agrees.

samreid commented 6 years ago

All remarks in https://github.com/phetsims/perennial/issues/53#issuecomment-352089058 sound great, I am optimistic that we wouldn't need to disable it.

mattpen commented 6 years ago

I think to accomplish this we just need to add --brands=phet,phet-io to the build-server call in rosetta

jonathanolson commented 6 years ago

Note that if phet-io isn't supported for the simulation, then including it in brands will fail out the build.

jbphet commented 6 years ago

@jonathanolson's comment seems like a deal breaker for adding these flags to the build requests that come from rosetta. @mattpen - true? Is there a good way through the metadata API for rosetta to determine whether phet-io is supported for a given sim?

mattpen commented 6 years ago

@jbphet - I don't think we want to include phet-io data in the Metadata API.

@jonathanolson - How would we be able to determine if a sim supports phet-io? Do we keep a list in phet-info, or is it indicated in package.json?

One option would be to make two calls to the build-server per translation, one per brand, instead of one call for both brands. We would then handle failures for phet somehow but ignore failures for phet-io.

jonathanolson commented 6 years ago

@jonathanolson - How would we be able to determine if a sim supports phet-io? Do we keep a list in phet-info, or is it indicated in package.json?

Yes, it would be included in the supportedBrands in the package.json (phet.supportedBrands.includes( 'phet-io'))

mattpen commented 6 years ago

Yes, it would be included in the supportedBrands in the package.json (phet.supportedBrands.includes( 'phet-io'))

It sounds like we should be checking this before the build step on the build-server, even if its already being checked locally via grunt. Should we (1) stop execution of the deploy and print an error to the logs or (2) only build the supported brands and print a warning to the logs?

@jbphet @jonathanolson - thoughts?

jonathanolson commented 6 years ago

Should we (1) stop execution of the deploy and print an error to the logs

That sounds the safest.

jbphet commented 6 years ago

@mattpen said:

I think to accomplish this we just need to add --brands=phet,phet-io to the build-server call in rosetta

and also:

I don't think we want to include phet-io data in the Metadata API.

and additionally:

It sounds like we should be checking this before the build step on the build-server, even if its already being checked locally via grunt. Should we (1) stop execution of the deploy and print an error to the logs or (2) only build the supported brands and print a warning to the logs?

So, unless I'm missing something, rosetta would have no way of determining whether a sim supports phet-io, but is supposed to request it as a build product, and under proposal 1 this would cause a failure and the translation wouldn't get published. So proposal 2 seems more viable, but I don't like that either since it will frequently cause warnings in the log. I would prefer that either there is a way for rosetta to make smarter requests about whether to build phet-io (perhaps reading the appropriate file out of the chipper or perennial repo on GitHub), or having a specific translation build request so that the build server knows what needs to be built.

jonathanolson commented 6 years ago

rosetta would have no way of determining whether a sim supports phet-io

If true, we could potentially support something like --brands=*, right?

mattpen commented 6 years ago

@jbphet - Rosetta should have access to the package.json file for the sims right? It could determine which brands to build based on that file.

jbphet commented 6 years ago

I'm trying not to assume that rosetta has other things around it on the file system, but it could read the package.json directly from GitHub and use that to decide. To do this right, though, it would have to pull that file off of the branch that matches the current version, which is doable. I'll create an issue and assign this to myself.

jbphet commented 6 years ago

An issue to add this to rosetta has been created, see https://github.com/phetsims/rosetta/issues/172.