phetsims / perennial

Maintenance tools that won't change with different versions of chipper checked out
MIT License
2 stars 5 forks source link

Missing support for parallel checkouts #330

Closed chrisklus closed 1 year ago

chrisklus commented 1 year ago

@zepumph and I were investigating more build failures from this morning and from that found some more places that weren't updated to support parallel checkouts:

https://github.com/phetsims/perennial/blob/62aeddbb4bc4b917da508c24e7b59c052eadbe9b/js/build-server/deployImages.js#L17 https://github.com/phetsims/perennial/blob/1250823a8ab7a4c37466351c17f92abd59295bfd/js/common/getDependencyRepos.js#L22 https://github.com/phetsims/perennial/blob/f4b90c827a938038ee2968537c972b9d2d13e27d/js/build-server/parseScreenNames.js#L18 https://github.com/phetsims/perennial/blob/2977b6222cc4c589cd3fe99fa58a513ff786ed49/js/build-server/pullMaster.js#L26

Assigning to @chrisklus and @mattpen to look into and we'll discuss how the review process for all of these types of changes should proceed/if we should even keep going without being advised by JO.

The bug in getDependencyRepos is a big problem - we think every MR that has gone out has used the dependencies.json from master instead of the release branch. EDIT: @zepumph clarified that the case we found was just getting the names of the dependency repos, not the SHAs, but there would still be problems if the release branch has difference dependencies than master. getDependencies also looks buggy for build server use, though.

mattpen commented 1 year ago

perennial/js/build-server/deployImages.js

This should run from master, at least in the current paradigm

zepumph commented 1 year ago

To know that we are solving this completely, I recommend creating a "master" release branch, under build-server/release-branches/master or something like that, and having only 2 directories in builder-server/: perennial and release-branches.

chrisklus commented 1 year ago

Thanks, we are currently trying out a new structure similar to that on phet-server-dev

mattpen commented 1 year ago

We are making progress on changes, but there are still a few hiccups. We're going to pick this up again tomorrow.

Successful tests (conducted on phet-server-dev):

Remaining Tests:

chrisklus commented 1 year ago

@zepumph could you please take a look at the build output for the following dev, RC, and production versions of phet-io-test-sim? We think the htaccess files are what we are most concerned about checking here.

dev https://phet-dev.colorado.edu/html/phet-io-test-sim/2.13.0-dev.2/phet-io/

rc https://phet-dev.colorado.edu/html/phet-io-test-sim/2.12.7-rc.1/phet-io/

prod https://ox-dev.colorado.edu/sims/html/phet-io-test-sim/2.12.7/ https://phet-io-dev.colorado.edu/sims/phet-io-test-sim/2.12/ (old password to see this)

chrisklus commented 1 year ago

@jonathanolson could you please review changes in the js/common directory from https://github.com/phetsims/perennial/commit/a3510b9d2c68042d81f724bd83a585d8ef514892 and https://github.com/phetsims/perennial/commit/5cfb8d17503ac5aae1cf926dcbb47f1ecf28758d? Please let us know if you would like to pair or discuss more. Thanks!

zepumph commented 1 year ago

This file looks password protected still, which shouldn't be the case since https://github.com/phetsims/perennial/commit/34db01f25b7de51a8bb71c674787250d7ff7d148 https://phet-io-dev.colorado.edu/sims/phet-io-test-sim/2.12/phet-io-test-sim-phet-io-api.json

I'm not sure why. That should be public, as it is in the dev and rc versions I tested. Otherwise it looks good.

mattpen commented 1 year ago

@zepumph - it looks like that change has been applied, but maybe isn't working as intended?

[mape5853@phet-server-dev 2.12.7]$ cat .htaccess 
<FilesMatch "(index\.\w+)$">

AuthType Basic
AuthName "PhET-iO Password Protected Area"
AuthUserFile /etc/httpd/conf/phet-io_pw
<LimitExcept OPTIONS>
  Require valid-user
</LimitExcept>
</FilesMatch>

# Editing these directly is not supported and will be overwritten by maintenance releases. Please change by modifying 
# the sim's package.json allowPublicAccess flag followed by a re-deploy.
# Satisfy Any
# Allow from all
[mape5853@phet-server-dev 2.12.7]$ pwd
/data/web/static/phet-io/sims/phet-io-test-sim/2.12.7

Or perhaps it could be because all of phet-io-dev.colorado.edu is password protected? We have a root .htaccess with these contents:

[mape5853@phet-server-dev phet-io]$ cat .htaccess 
AuthType Basic
AuthName "PhET-iO Password Protected Area"
AuthUserFile /etc/httpd/conf/phet-io_pw
Require valid-user
Options +Indexes
[mape5853@phet-server-dev phet-io]$ pwd
/data/web/static/phet-io

EDIT: @chrisklus and I are pretty confident it is the root .htaccess file that is causing the problem. We commented out the Require valid-user line and then the api file was accessible without a password. We put the line back in place after testing.

mattpen commented 1 year ago

@chrisklus and I deleted the old "master" copies of the repos on phet-server2, then we deployed the build-server changes to production. We tested a deploy of phet-io-test-sim in both phet and phet-io brand and everything seems to be working well.

Next step is to do a code review for the parallel checkouts work.

chrisklus commented 1 year ago

We also triggered one of the failed translation builds (area-builder that had no dependencies.json in master) and that built successfully

mattpen commented 1 year ago

@chrisklus and I believe that things should be operating pretty well now. We deleted all of the old master checkouts on phet-server2, and there have been several successful deploys since then. We opened https://github.com/phetsims/perennial/issues/332 to review the changes, but believe this issue can be closed.