rusackas opened a new pull request, #45606:
URL: https://github.com/apache/superset/pull/45606

   ### SUMMARY
   Every CI step that runs inside the `superset-node-ci` image opens with `npm 
i`, and in the logs that line reads `added 104 packages in 5m`. It happens 
twice in `lint-frontend` (the lint step and the tsc step), and once each in 
`bundle-size`, `chromatic` and the nightly bundle job. On `lint-frontend` 
that's ten of the job's thirteen minutes doing nothing useful.
   
   The cause is in the Dockerfile: the `npm ci` layer bind-mounts only the root 
`package.json` and lockfile, so npm never sees the workspace manifests and 
skips the `generator-superset` workspace's devDependencies. Reproduced locally 
by building master's image and diffing the installed packages around `npm i`: 
it adds 123 packages, all under `yeoman-generator`, `yosay`, `mem-fs-editor`, 
`yeoman-test`, `simple-git` and their dependencies. The Dockerfile comment 
already acknowledged that COPYing just the manifests wasn't straightforward.
   
   This adds a small `superset-node-manifests` stage that copies 
`superset-frontend` and prunes it to `package.json` and `package-lock.json` 
files, COPYs that into the CI stage before `npm ci`, and removes the per-step 
`npm i` from the five places it appears. In the fixed image, `npm i` is a 
5-second no-op with the `@superset-ui/*` workspace links and the packages' 
nested `node_modules` in place, so the jobs no longer need it.
   
   One side effect to know about: the `npm ci` layer is now keyed on all 
workspace manifests instead of just the root two, so a workspace `package.json` 
change (e.g. a version bump across packages) invalidates it where it previously 
wouldn't have. That's a correct invalidation; before, the layer could be stale 
for exactly those changes.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   N/A
   
   ### TESTING INSTRUCTIONS
   `docker buildx build --target superset-node-ci -t node-ci . && docker run 
--rm node-ci npm i` should report nothing added and finish in seconds. On the 
PR, compare the `lint-frontend` job (its "lint" and "tsc" steps in particular) 
against a recent master run; each should lose about five minutes. I'll post the 
numbers here.
   
   ### ADDITIONAL INFORMATION
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration (follow approval process in 
[SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to