Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use chg in LfMerge, only in local dev LF for now #1842

Merged
merged 2 commits into from
Feb 6, 2025

Conversation

rmunn
Copy link
Collaborator

@rmunn rmunn commented Feb 3, 2025

Fixes #1841

Description

We'll start using chg for LfMerge in local LF development; once we know that it works, we'll add the relevant environment variable to the k8s deployment so that staging and then prod also start getting the same Send/Receive speedups.

Checklist

  • I have labeled my PR with: bug, feature, engineering, security fix or testing
  • I have performed a self-review of my own code
  • I have reviewed the title & description of this PR which I will use as the squashed PR commit message
  • I have commented my code, particularly in hard-to-understand areas
  • I have added tests that prove my fix is effective or that my feature works
  • I have enabled auto-merge (optional)

Testing

  • Run some Send/Receives on local LF and make sure everything still works.

@rmunn rmunn added the engineering Tasks which do not directly relate to a user-facing feature or fix label Feb 3, 2025
@rmunn rmunn self-assigned this Feb 3, 2025
Copy link

github-actions bot commented Feb 3, 2025

Unit Test Results

362 tests   362 ✅  13s ⏱️
 37 suites    0 💤
  1 files      0 ❌

Results for commit 223c2fa.

♻️ This comment has been updated with latest results.

@hahn-kev
Copy link
Collaborator

hahn-kev commented Feb 4, 2025

looks like our PNPM install is failing in LF too, would you mind looking into that? I'm fine if you merge this PR without fixing it as they're totally unrelated

@rmunn
Copy link
Collaborator Author

rmunn commented Feb 4, 2025

looks like our PNPM install is failing in LF too, would you mind looking into that? I'm fine if you merge this PR without fixing it as they're totally unrelated

Huh. So there's some kind of problem with NPM's signing infrastructure and it's unrelated to the specific PNPM 9.15.5 release (LF is still on 9.11.0). Weird; hopefully they sort it out soon because that's going to affect millions of projects.

I'll try rerunning the failed GHA builds once in case it's already been sorted out, then if it's still failing across the board I'll just merge this.

@rmunn
Copy link
Collaborator Author

rmunn commented Feb 4, 2025

@rmunn
Copy link
Collaborator Author

rmunn commented Feb 4, 2025

Corepack bug present in the corepack versions distributed with NPM (corepack 0.30.0). Running npm install --global corepack@latest before running corepack enable will update corepack to 0.31.0 which has the bugfix. (PNPM rotated a key, so for a while both the old and new keys were returned from the "send me the key" API endpoint, and IIUC corepack 0.30.0 didn't handle that correctly and only checked against the first key returned, which was the old key.

@rmunn
Copy link
Collaborator Author

rmunn commented Feb 4, 2025

That solved the PNPM install issue, only to run into a different issue: "Package libasound2 has no installation candidate" which trying to install browsers for pnpm exec playwright install. I'll work on solving that separately, but I'll go ahead and merge this PR since that error is unrelated to this one and is probably already present on develop.

@rmunn rmunn merged commit d4a8890 into develop Feb 6, 2025
4 of 17 checks passed
@rmunn rmunn deleted the chore/use-chg-in-lfmerge branch February 6, 2025 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
engineering Tasks which do not directly relate to a user-facing feature or fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LF: Use chg in LfMerge, only in local dev for now
2 participants