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

test: add env ATUIN_TEST_LOCAL_TIMEOUT to control test timeout of SQLite #2337

Merged
merged 2 commits into from
Aug 5, 2024

Conversation

jaxvanyang
Copy link
Contributor

This make it possible to control the timeout of SQLite operations in test. And ATUIN_TEST_LOCAL_TIMEOUT defaults to the default local_timeout, which is actually used in the client. Instead of a small timeout (0.1), this change makes the test less likely to fail and better imitate the default behavior.

SQLite operation timeout was first introduced from #1590, including connection and store timeout. The env ATUIN_TEST_SQLITE_STORE_TIMEOUT which added by #1703 only specify the store timeout. This commit doesn't deprecate ATUIN_TEST_SQLITE_STORE_TIMEOUT, but control it by setting its default to the new env ATUIN_TEST_LOCAL_TIMEOUT.

The reason is the same as #1703, but this time it fails due to connection timeout: archriscv.felixc.at/.status/log.htm?url=logs/atuin/atuin-18.3.0-1.log.

And I have two question for this commit if it's acceptable:

  • Should we replace the old ATUIN_TEST_SQLITE_STORE_TIMEOUT with the new ATUIN_TEST_LOCAL_TIMEOUT for simplicity?
  • Is there a simple way to get the local_timeout value? Using Settings::builder() seems complex.

Checks

  • I am happy for maintainers to push small adjustments to this PR, to speed up the review cycle
  • I have checked that there are no existing pull requests for the same thing

This make it possible to control the timeout of SQLite operations in
test. And ATUIN_TEST_LOCAL_TIMEOUT defaults to the default local_timeout,
which is actually used in the client. Instead of a small timeout (0.1),
this change makes the test less likely to fail and better imitate the
default behavior.

SQLite operation timeout was first introduced from atuinsh#1590, including
connection and store timeout. The env ATUIN_TEST_SQLITE_STORE_TIMEOUT
which added by atuinsh#1703 only specify the store timeout. This commit doesn't
deprecate ATUIN_TEST_SQLITE_STORE_TIMEOUT, but control it by setting its
default to the new env ATUIN_TEST_LOCAL_TIMEOUT.
jaxvanyang added a commit to jaxvanyang/archriscv-packages that referenced this pull request Aug 2, 2024
This commit increases the SQLite operation timeout to 2.0 to make the
test pass. Details see upstream: atuinsh/atuin#2337.
jaxvanyang added a commit to jaxvanyang/archriscv-packages that referenced this pull request Aug 2, 2024
This commit increases the SQLite operation timeout to 2s to make the
test pass. Details see upstream: atuinsh/atuin#2337.
@ellie
Copy link
Member

ellie commented Aug 2, 2024

Should we replace the old ATUIN_TEST_SQLITE_STORE_TIMEOUT with the new ATUIN_TEST_LOCAL_TIMEOUT for simplicity?

That would be good!

Is there a simple way to get the local_timeout value? Using Settings::builder() seems complex.

Not really. The builder is complex because the setting could come from multiple sources - env, config file, others in the future. I'd like to overhaul it a bit at some point though

Otherwise, how does SQLite tend to perform on RISC? Totally cool with this change, but I'm just wondering if reads are always going to take >100ms even for simple cases

@jaxvanyang
Copy link
Contributor Author

Should we replace the old ATUIN_TEST_SQLITE_STORE_TIMEOUT with the new ATUIN_TEST_LOCAL_TIMEOUT for simplicity?

That would be good!

OK, I'll push another commit for that.

Otherwise, how does SQLite tend to perform on RISC? Totally cool with this change, but I'm just wondering if reads are always going to take >100ms even for simple cases

Short answer: not always. RISC-V is just an ISA, performance is more related to silicon technology. AFAIK, RISC-V is at its early stage, most companies focus on embedded system, providing SBCs for testing. These devices are not powerful. And Arch Linux RISC-V is just a personal port project, although it's sponsored, I think its builder machines have limited performance, I think they are using QEMU for some builds, it may cause some performance problem as well. If you are interested in current performance status of RISC-V, you can find some articles on Phoronix, e.g., Benchmarking The First RISC-V Cloud Server: Scaleway EM-RV1 Performance - Phoronix.

…TIMEOUT

This deprecate ATUIN_TEST_SQLITE_STORE_TIMEOUT for simplicity as the new
env ATUIN_TEST_LOCAL_TIMEOUT can control both connection and store
timeout of SQLite in test. Details see 4d88611.

Revert: atuinsh#1703.
Copy link
Member

@ellie ellie left a comment

Choose a reason for hiding this comment

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

thank you 🙏

Seeing as this is your first time contributing, if you would like a holographic contributors-only Atuin sticker, then please fill out this form!

We do also have a Discord if you'd like to ask any questions, or just fancy hanging out!

@ellie ellie merged commit 90e7d28 into atuinsh:main Aug 5, 2024
19 checks passed
@jaxvanyang jaxvanyang deleted the jax/test/timeout branch August 5, 2024 14:02
@jaxvanyang
Copy link
Contributor Author

thank you 🙏

Seeing as this is your first time contributing, if you would like a holographic contributors-only Atuin sticker, then please fill out this form!

We do also have a Discord if you'd like to ask any questions, or just fancy hanging out!

Glad to hear that, thank you.

felixonmars pushed a commit to felixonmars/archriscv-packages that referenced this pull request Aug 7, 2024
This commit increases the SQLite operation timeout to 2s to make the
test pass. Details see upstream: atuinsh/atuin#2337.
tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request Dec 28, 2024
This MR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [atuinsh/atuin](https://github.com/atuinsh/atuin) | minor | `v18.3.0` -> `v18.4.0` |

MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot).

**Proposed changes to behavior should be submitted there as MRs.**

---

### Release Notes

<details>
<summary>atuinsh/atuin (atuinsh/atuin)</summary>

### [`v18.4.0`](https://github.com/atuinsh/atuin/releases/tag/v18.4.0)

[Compare Source](atuinsh/atuin@v18.3.0...v18.4.0)

#### Theme system

Thanks to [@&#8203;philtweir](https://github.com/philtweir), we now have a theming system! It's too much to write about here, so see the section in the docs: https://docs.atuin.sh/guide/theming/

#### Default changes

With this release, some defaults have changed. The most obvious will likely be the switch to the compact UI style + inline rendering

This can be reverted by setting

    style = "full"
    inline_height = 0

Read more here: atuinsh/atuin#2249

#### `atuin wrapped`

See your stats for 2024 with `atuin wrapped`

![CleanShot 2024-12-27 at 17 10 45@&#8203;2x](https://github.com/user-attachments/assets/c7853c47-4e3e-49c7-b811-c0a62ab8c259)

#### Build changes

Thanks to [@&#8203;senekor](https://github.com/senekor), we no longer require protoc available at build time, and instead use `protox`

##### Bug Fixes

-   *(crate)* Add missing description ([#&#8203;2106](atuinsh/atuin#2106))
-   *(crate)* Add description to daemon crate ([#&#8203;2107](atuinsh/atuin#2107))
-   *(daemon)* Add context to error when unable to connect ([#&#8203;2394](atuinsh/atuin#2394))
-   *(deps)* Pin tiny_bip to 1.0.0 until breaking change resolved ([#&#8203;2412](atuinsh/atuin#2412))
-   *(docker)* Update Dockerfile ([#&#8203;2369](atuinsh/atuin#2369))
-   *(history)* Logic for store_failed=false ([#&#8203;2284](atuinsh/atuin#2284))
-   *(mail)* Incorrect alias and error logs ([#&#8203;2346](atuinsh/atuin#2346))
-   *(mail)* Enable correct tls features for postmark client ([#&#8203;2347](atuinsh/atuin#2347))
-   *(theme)* Restore original colours ([#&#8203;2339](atuinsh/atuin#2339))
-   *(themes)* Restore default theme, refactor ([#&#8203;2294](atuinsh/atuin#2294))
-   *(tui)* Press ctrl-a twice should jump to beginning of line ([#&#8203;2246](atuinsh/atuin#2246))
-   *(tui)* Don't panic when search result is empty and up is pressed ([#&#8203;2395](atuinsh/atuin#2395))
-   Cargo binstall config ([#&#8203;2112](atuinsh/atuin#2112))
-   Unitless sync_frequence = 0 not parsed by humantime ([#&#8203;2154](atuinsh/atuin#2154))
-   Some --help comments didn't show properly ([#&#8203;2176](atuinsh/atuin#2176))
-   Ensure we cleanup all tables when deleting ([#&#8203;2191](atuinsh/atuin#2191))
-   Add idx cache unique index ([#&#8203;2226](atuinsh/atuin#2226))
-   Idx cache inconsistency ([#&#8203;2231](atuinsh/atuin#2231))
-   Ambiguous column name ([#&#8203;2232](atuinsh/atuin#2232))
-   Atuin-daemon optional dependency ([#&#8203;2306](atuinsh/atuin#2306))
-   Windows build error ([#&#8203;2321](atuinsh/atuin#2321))
-   Codespell config still references the ui ([#&#8203;2330](atuinsh/atuin#2330))
-   Remove dbg! macro ([#&#8203;2355](atuinsh/atuin#2355))
-   Disable mail by default, resolve [#&#8203;2404](atuinsh/atuin#2404) ([#&#8203;2405](atuinsh/atuin#2405))
-   Time offset display in `atuin status` ([#&#8203;2433](atuinsh/atuin#2433))
-   Disable the actuated mirror on the x86 docker builder ([#&#8203;2443](atuinsh/atuin#2443))

##### Documentation

-   *(README)* Fix broken link ([#&#8203;2206](atuinsh/atuin#2206))
-   Streamline readme ([#&#8203;2203](atuinsh/atuin#2203))
-   Update quickstart install command ([#&#8203;2205](atuinsh/atuin#2205))

##### Features

-   *(bash/blesh)* Hook into BLE_ONLOAD to resolve loading order issue ([#&#8203;2234](atuinsh/atuin#2234))
-   *(client)* Add filter mode enablement and ordering configuration ([#&#8203;2430](atuinsh/atuin#2430))
-   *(daemon)* Follow XDG_RUNTIME_DIR if set ([#&#8203;2171](atuinsh/atuin#2171))
-   *(history)* Filter out various environment variables containing potential secrets ([#&#8203;2174](atuinsh/atuin#2174))
-   *(tui)* Configurable prefix character ([#&#8203;2157](atuinsh/atuin#2157))
-   *(tui)* Customizable Themes ([#&#8203;2236](atuinsh/atuin#2236))
-   *(tui)* Fixed preview height option ([#&#8203;2286](atuinsh/atuin#2286))
-   Use cargo-dist installer from our install script ([#&#8203;2108](atuinsh/atuin#2108))
-   Add user account verification ([#&#8203;2190](atuinsh/atuin#2190))
-   Add GitLab PAT to secret patterns ([#&#8203;2196](atuinsh/atuin#2196))
-   Add several other GitHub access token patterns ([#&#8203;2200](atuinsh/atuin#2200))
-   Add npm, Netlify and Pulumi tokens to secret patterns ([#&#8203;2210](atuinsh/atuin#2210))
-   Allow advertising a fake version to clients ([#&#8203;2228](atuinsh/atuin#2228))
-   Monitor idx cache consistency before switching ([#&#8203;2229](atuinsh/atuin#2229))
-   Ultracompact Mode (search-only) ([#&#8203;2357](atuinsh/atuin#2357))
-   Right Arrow to modify selected command ([#&#8203;2453](atuinsh/atuin#2453))
-   Provide additional clarity around key management ([#&#8203;2467](atuinsh/atuin#2467))
-   Add `atuin wrapped` ([#&#8203;2493](atuinsh/atuin#2493))

##### Miscellaneous Tasks

-   *(build)* Compile protobufs with protox ([#&#8203;2122](atuinsh/atuin#2122))
-   *(ci)* Do not run current ci for ui ([#&#8203;2189](atuinsh/atuin#2189))
-   *(ci)* Codespell again ([#&#8203;2332](atuinsh/atuin#2332))
-   *(install)* Use posix sh, not bash ([#&#8203;2204](atuinsh/atuin#2204))
-   *(nix)* De-couple atuin nix build from nixpkgs rustc version ([#&#8203;2123](atuinsh/atuin#2123))
-   Add installer e2e tests ([#&#8203;2110](atuinsh/atuin#2110))
-   Remove unnecessary proto import ([#&#8203;2120](atuinsh/atuin#2120))
-   Update to rust 1.78
-   Add audit config, ignore RUSTSEC-2023-0071 ([#&#8203;2126](atuinsh/atuin#2126))
-   Setup dependabot for the ui ([#&#8203;2128](atuinsh/atuin#2128))
-   Cargo and pnpm update ([#&#8203;2127](atuinsh/atuin#2127))
-   Update to rust 1.79 ([#&#8203;2138](atuinsh/atuin#2138))
-   Update to cargo-dist 0.16, enable attestations ([#&#8203;2156](atuinsh/atuin#2156))
-   Do not use package managers in installer ([#&#8203;2201](atuinsh/atuin#2201))
-   Enable record sync by default ([#&#8203;2255](atuinsh/atuin#2255))
-   Remove ui directory ([#&#8203;2329](atuinsh/atuin#2329))
-   Update to rust 1.80 ([#&#8203;2344](atuinsh/atuin#2344))
-   Update rust to `1.80.1` ([#&#8203;2362](atuinsh/atuin#2362))
-   Enable inline height and compact by default ([#&#8203;2249](atuinsh/atuin#2249))
-   Update to rust 1.82 ([#&#8203;2432](atuinsh/atuin#2432))
-   Update cargo-dist ([#&#8203;2471](atuinsh/atuin#2471))

##### Performance

-   *(search)* Benchmark smart sort ([#&#8203;2202](atuinsh/atuin#2202))
-   Create idx cache table ([#&#8203;2140](atuinsh/atuin#2140))
-   Write to the idx cache ([#&#8203;2225](atuinsh/atuin#2225))

##### Testing

-   Add env ATUIN_TEST_LOCAL_TIMEOUT to control test timeout of SQLite ([#&#8203;2337](atuinsh/atuin#2337))

#### Install atuin 18.4.0

##### Install prebuilt binaries via shell script

```sh
curl --proto '=https' --tlsv1.2 -LsSf https://github.com/atuinsh/atuin/releases/download/v18.4.0/atuin-installer.sh | sh
```

#### Download atuin 18.4.0

|  File  | Platform | Checksum |
|--------|----------|----------|
| [atuin-aarch64-apple-darwin.tar.gz](https://github.com/atuinsh/atuin/releases/download/v18.4.0/atuin-aarch64-apple-darwin.tar.gz) | Apple Silicon macOS | [checksum](https://github.com/atuinsh/atuin/releases/download/v18.4.0/atuin-aarch64-apple-darwin.tar.gz.sha256) |
| [atuin-x86\_64-apple-darwin.tar.gz](https://github.com/atuinsh/atuin/releases/download/v18.4.0/atuin-x86\_64-apple-darwin.tar.gz) | Intel macOS | [checksum](https://github.com/atuinsh/atuin/releases/download/v18.4.0/atuin-x86\_64-apple-darwin.tar.gz.sha256) |
| [atuin-aarch64-unknown-linux-gnu.tar.gz](https://github.com/atuinsh/atuin/releases/download/v18.4.0/atuin-aarch64-unknown-linux-gnu.tar.gz) | ARM64 Linux | [checksum](https://github.com/atuinsh/atuin/releases/download/v18.4.0/atuin-aarch64-unknown-linux-gnu.tar.gz.sha256) |
| [atuin-x86\_64-unknown-linux-gnu.tar.gz](https://github.com/atuinsh/atuin/releases/download/v18.4.0/atuin-x86\_64-unknown-linux-gnu.tar.gz) | x64 Linux | [checksum](https://github.com/atuinsh/atuin/releases/download/v18.4.0/atuin-x86\_64-unknown-linux-gnu.tar.gz.sha256) |
| [atuin-aarch64-unknown-linux-musl.tar.gz](https://github.com/atuinsh/atuin/releases/download/v18.4.0/atuin-aarch64-unknown-linux-musl.tar.gz) | ARM64 MUSL Linux | [checksum](https://github.com/atuinsh/atuin/releases/download/v18.4.0/atuin-aarch64-unknown-linux-musl.tar.gz.sha256) |
| [atuin-x86\_64-unknown-linux-musl.tar.gz](https://github.com/atuinsh/atuin/releases/download/v18.4.0/atuin-x86\_64-unknown-linux-musl.tar.gz) | x64 MUSL Linux | [checksum](https://github.com/atuinsh/atuin/releases/download/v18.4.0/atuin-x86\_64-unknown-linux-musl.tar.gz.sha256) |

#### Verifying GitHub Artifact Attestations

The artifacts in this release have attestations generated with GitHub Artifact Attestations. These can be verified by using the [GitHub CLI](https://cli.github.com/manual/gh_attestation_verify):

```sh
gh attestation verify <file-path of downloaded artifact> --repo atuinsh/atuin
```

You can also download the attestation from [GitHub](https://github.com/atuinsh/atuin/attestations) and verify against that directly:

```sh
gh attestation verify <file-path of downloaded artifact> --bundle <file-path of downloaded attestation>
```

#### New Contributors

-   [@&#8203;senekor](https://github.com/senekor) made their first contribution in atuinsh/atuin#2122
-   [@&#8203;injust](https://github.com/injust) made their first contribution in atuinsh/atuin#2166
-   [@&#8203;davidolrik](https://github.com/davidolrik) made their first contribution in atuinsh/atuin#2196
-   [@&#8203;julienp](https://github.com/julienp) made their first contribution in atuinsh/atuin#2210
-   [@&#8203;eth3lbert](https://github.com/eth3lbert) made their first contribution in atuinsh/atuin#2246
-   [@&#8203;philtweir](https://github.com/philtweir) made their first contribution in atuinsh/atuin#2236
-   [@&#8203;lucacome](https://github.com/lucacome) made their first contribution in atuinsh/atuin#2257
-   [@&#8203;JRGould](https://github.com/JRGould) made their first contribution in atuinsh/atuin#2284
-   [@&#8203;jaxvanyang](https://github.com/jaxvanyang) made their first contribution in atuinsh/atuin#2337
-   [@&#8203;cultpony](https://github.com/cultpony) made their first contribution in atuinsh/atuin#2369
-   [@&#8203;Reverier-Xu](https://github.com/Reverier-Xu) made their first contribution in atuinsh/atuin#2433
-   [@&#8203;bboynton97](https://github.com/bboynton97) made their first contribution in atuinsh/atuin#2453
-   [@&#8203;pamburus](https://github.com/pamburus) made their first contribution in atuinsh/atuin#2430

**Full Changelog**: atuinsh/atuin@v18.3.0...v18.4.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOS44My4zIiwidXBkYXRlZEluVmVyIjoiMzkuODMuMyIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiUmVub3ZhdGUgQm90Il19-->
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