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 fresher node and debian version #1255

Merged
merged 18 commits into from
Oct 10, 2024
Merged

use fresher node and debian version #1255

merged 18 commits into from
Oct 10, 2024

Conversation

paulfitz
Copy link
Member

@paulfitz paulfitz commented Oct 9, 2024

Context

This moves to node 22 and debian bookworm, since the versions we've been building and testing with are getting old.

There is some old material kept around for (speaks very quietly) Python 2 (looks around hoping no-one heard) which we continue to support for some long-time users but really really should drop soon.

Has this been tested?

Existing tests should pass before this lands. I did spot checks locally. I tested the docker build locally for Python3 and Python2.

The changes for the node upgrade were all test related. I did them in a way that shouldn't impair running on older versions of node, and did spot checks for this. This is to give some breathing room for upgrading Grist Lab's grist-saas as follow up work.

@paulfitz paulfitz marked this pull request as ready for review October 10, 2024 13:37
@georgegevoian georgegevoian self-requested a review October 10, 2024 14:29
Dockerfile Outdated Show resolved Hide resolved
Copy link
Contributor

@georgegevoian georgegevoian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for me on arm64. Thanks @paulfitz!

@georgegevoian
Copy link
Contributor

For the Importer failure, I think it might need a gu.waitForServer() between gu.sendKeys(Key.ESCAPE) and waitForDiffPreviewToLoad().

@paulfitz
Copy link
Member Author

For the Importer failure, I think it might need a gu.waitForServer() between gu.sendKeys(Key.ESCAPE) and waitForDiffPreviewToLoad().

Thanks I'll try. I disrupted things when, running the tests locally, I needed to set them in light mode since colors are checked literally :(

@georgegevoian
Copy link
Contributor

For the Importer failure, I think it might need a gu.waitForServer() between gu.sendKeys(Key.ESCAPE) and waitForDiffPreviewToLoad().

Thanks I'll try. I disrupted things when, running the tests locally, I needed to set them in light mode since colors are checked literally :(

Yeah, I've been bit by that before too. Could do with a hook before browser tests are run that disables syncing with OS in preferences.

@paulfitz paulfitz merged commit aa69652 into main Oct 10, 2024
11 checks passed
@paulfitz paulfitz deleted the paulfitz/node-22-bookworm branch October 10, 2024 20:59
paulfitz added a commit that referenced this pull request Oct 12, 2024
This partially undos a recent upgrade because it results in
`gvisor` sandboxing not functioning. Hopefully this is temporary,
to give time to track down the cause, fix it, and extend CI testing
on candidate docker images to include `gvisor` sandboxing.

Upgrade: #1255

Sorry for the breakage!
paulfitz added a commit that referenced this pull request Oct 12, 2024
This partially undos a recent upgrade because it results in
`gvisor` sandboxing not functioning. Hopefully this is temporary,
to give time to track down the cause, fix it, and extend CI testing
on candidate docker images to include `gvisor` sandboxing.

Upgrade: #1255

Sorry for the breakage!
paulfitz added a commit that referenced this pull request Nov 18, 2024
This returns to an upgrade first attempted in:
  #1255
That upgrade ran into sandbox trouble, which eventually proved to
be a small change in the layout of directories in bookworm relative
to buster (`/lib64` became a symlink).
paulfitz added a commit that referenced this pull request Nov 18, 2024
This returns to an upgrade first attempted in:
  #1255
That upgrade ran into sandbox trouble, which eventually proved to
be a small change in the layout of directories in bookworm relative
to buster (`/lib64` became a symlink).
paulfitz added a commit that referenced this pull request Nov 19, 2024
This returns to an upgrade first attempted in:
  #1255
That upgrade ran into sandbox trouble, which eventually proved to
be a small change in the layout of directories in bookworm relative
to buster (`/lib64` became a symlink).
hexaltation added a commit to hexaltation/grist-core that referenced this pull request Jan 7, 2025
paulfitz pushed a commit that referenced this pull request Jan 7, 2025
## Context

In #1255 node version was updated to node 22 in CI.
In #1311 node version was updated to node 22 in `dockerfile`
It seems a good idea to upgrade `.nvmrc` to such node version too.

## Proposed solution

Update of value in `.nvmrc`

## Has this been tested?

- [ ] 👍 yes, I added tests to the test suite
- [ ] 💭 no, because this PR is a draft and still needs work
- [X] 🙅 no, because this is not relevant here
- [ ] 🙋 no, because I need help <!-- Detail how we can help you -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants