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

Bug: Dorothy installation can fail if GitHub API Rate Limit is encountered again after the wait period #270

Open
edmcman opened this issue Dec 29, 2024 · 14 comments
Assignees
Labels
bug Broken or unexpected

Comments

@edmcman
Copy link

edmcman commented Dec 29, 2024

This is not a great install experience:

Image

It's a little hard to see, but note the 16 minutes remaining

@edmcman
Copy link
Author

edmcman commented Dec 29, 2024

After waiting, it failed anyway Image

Now it's stuck with zero information about what it's doing. I can tell from top that it's compiling rust crates, but more visibility from the installer would be nice.

@edmcman
Copy link
Author

edmcman commented Dec 29, 2024

Also (sorry for piling on with constructive criticism)... it's really not great that deno is being built in the first place. I'm installing on a raspberry pi. It's been building deno for over 10 minutes now.

Installing Dorothy should ideally be:

  1. Fast
  2. Minimal -- I don't want really want deno or rust installed.
  3. Small. Rust and cargo is using 4.2 GiB and it's not even complete.
  4. Transparent. I should know everything that the install process is doing before it happens.

Again, I apologize for all the criticism. But I really just wanted to install dorothy quickly to gain access to one of my custom commands. This was not what I expected or wanted at all! Perhaps speed and disk space used could also be measured by the CI process and considered.

@edmcman edmcman changed the title Install experience: Stuck waiting for GitHub API Rate Limit to reset on install! Install experience: Very slow, with long and large undesired dependency builds Dec 29, 2024
@edmcman
Copy link
Author

edmcman commented Dec 29, 2024

Eventually the whole thing failed:

error[E0433]: failed to resolve: use of undeclared crate or module `flate2`
  --> /home/ed/.cargo/registry/src/index.crates.io-6f17d22bba15001f/zip-2.2.2/src/write.rs:35:5
   |
35 | use flate2::{write::DeflateEncoder, Compression};
   |     ^^^^^^ use of undeclared crate or module `flate2`

error[E0432]: unresolved import `flate2`
  --> /home/ed/.cargo/registry/src/index.crates.io-6f17d22bba15001f/zip-2.2.2/src/write.rs:35:5
   |
35 | use flate2::{write::DeflateEncoder, Compression};
   |     ^^^^^^ use of undeclared crate or module `flate2`

error[E0433]: failed to resolve: use of undeclared crate or module `flate2`
   --> /home/ed/.cargo/registry/src/index.crates.io-6f17d22bba15001f/zip-2.2.2/src/compression.rs:195:14
    |
195 |     Deflated(flate2::bufread::DeflateDecoder<R>),
    |              ^^^^^^ use of undeclared crate or module `flate2`

error[E0433]: failed to resolve: use of undeclared crate or module `flate2`
   --> /home/ed/.cargo/registry/src/index.crates.io-6f17d22bba15001f/zip-2.2.2/src/compression.rs:234:40
    |
234 |                 Decompressor::Deflated(flate2::bufread::DeflateDecoder::new(reader))
    |                                        ^^^^^^ use of undeclared crate or module `flate2`

Some errors have detailed explanations: E0432, E0433.
For more information about an error, try `rustc --explain E0432`.
error: could not compile `zip` (lib) due to 4 previous errors
error: failed to compile `deno v2.1.4`, intermediate artifacts can be found at `/home/ed/.cargo/target`.
To reuse those artifacts with a future compilation, set the environment variable `CARGO_TARGET_DIR` to that path.
The [deno] required utility was not installed via [cargo]
The [deno] required utility was not installed. ❌

Cargo and rust dirs are using 5.8 GiB.

@edmcman edmcman changed the title Install experience: Very slow, with long and large undesired dependency builds Install experience: Very slow, with long and large undesired dependency builds, and eventually fails Dec 29, 2024
@balupton
Copy link
Member

balupton commented Dec 29, 2024

/ref #147

Sorry for the horrible experience, it shouldn't be happening, and is not intentional, and Dorothy does everything to avoid this. It seems something went wrong with Deno's prebuilt binaries so Dorothy needing Deno tried to compile it itself. Dorothy needs to fix this.

As for why Deno is required: Earlier this year, once Deno provided prebuilt binaries for all our targets except riscv, I decided to use Deno to write Dorothy's echo-regexp command that uses Deno regular expressions to solve the strange configuration corruption edge cases that could occur previously, this eliminatex the dependency on several other rust packages with various cross-platform support: sd, teip, ripgrep.

As for why it was compiling: Deno is meant to support arm64, which is why I took the plunge to adopt it, so it seems somehow something when wrong with their official build and it had to build it itself, which failed. Compilation on raspberry pis is a painful long process, and the setup-util technology of Dorothy is designed to do everything possible to avoid that, able to try multiple options before installing a package system or trying compilation.

Which raspberry pi model was it? and which OS was it? Did you have sudo access in this case?

Our CI tests on arm, and I have several raspberry pi 5s and 4s. I'll see if I can reproduce.

Knowing these, I'll see if we can fix it inside Dorothy or if we have to coordinate with the Deno team.

There is also plunge I can make with Dorothy which is offering our own prebuilt binaries for dependencies for every platform, however that is significant work at this time. And at least in this case, it shouldn't have been needed.

@balupton
Copy link
Member

balupton commented Dec 29, 2024

Ohhhh. So the reason the prebuilt binaries failed to fetch was the github rate limit was hit, then for some reason was still exhausted after the wait period, so Dorothy was unable to scan the github repository release assets for a compatible download for the system....

Was this on a shared IP that does a lot of github stuff? As waiting the wait period for the reset should have worked, it's what our CI does when I push too many commits too quickly. However, as it waited then hit the same unauthorised error maybe something else continued to hit the github api during the wait period.

Perhaps we need to cache the github responses on our own cloud, be it as our own server or github actions automation. Or if the github rate limit is set, and if interactive, we present a choose menu with three options (1) authorise with github so we can continue the optimal path, (2) wait the desired time and try again, and if failing present the same menu instead of failing (3) give up on github api and continue to a fallback or failure

@edmcman
Copy link
Author

edmcman commented Dec 30, 2024

Which raspberry pi model was it? and which OS was it? Did you have sudo access in this case?

Raspberry Pi 5 Model B Rev 1.0
Ubuntu 24.04.1 LTS

I do have sudo access.

Our CI tests on arm, and I have several raspberry pi 5s and 4s. I'll see if I can reproduce.

I'm not sure why the build of deno failed. It's basically a fresh Ubuntu install.

Was this on a shared IP that does a lot of github stuff? As waiting the wait period for the reset should have worked, it's what our CI does when I push too many commits too quickly. However, as it waited then hit the same unauthorised error maybe something else continued to hit the github api during the wait period.

I'm unsure why the GitHub API/download failed. This was on my home network. As far as I know, our external IP is not shared with anyone else. There was not any GitHub API usage that I knew of.

The problem is still occurring. I'll try and determine what is going on there.

@edmcman
Copy link
Author

edmcman commented Dec 30, 2024

The rate limit seems to be right. I am not sure what is using the API though.. I generally don't use the API unauthenticated.

{"resources":{"core":{"limit":60,"remaining":0,"reset":1735530582,"used":60,"resource":"core"},"graphql":{"limit":0,"remaining":0,"reset":1735531523,"used":0,"resource":"graphql"},"integration_manifest":{"limit":5000,"remaining":5000,"reset":1735531523,"used":0,"resource":"integration_manifest"},"search":{"limit":10,"remaining":10,"reset":1735527983,"used":0,"resource":"search"}},"rate":{"limit":60,"remaining":0,"reset":1735530582,"used":60,"resource":"core"}}

@balupton
Copy link
Member

balupton commented Dec 30, 2024

Okay, I'll look into code changes to improve on this situation later this week, as I have other tasks to attend to.

For the meantime, extract https://github.com/denoland/deno/releases/download/v2.1.4/deno-aarch64-unknown-linux-gnu.zip and place the deno executable from it inside your ~/.local/bin then chmod +x ~/.local/bin/deno. To unzip tar -xvf ./deno-aarch64-unknown-linux-gnu.zip.

Then run the Dorothy install again and it should work, as it seems the install of jq went fine.

Besides the dependencies mentioned in the readme, which are essential even for the installer, the use of Deno was able to drop our minimal dependencies down to just jq and deno. jq was probably installed via apt on Debian or via github.com/jqlang/jq if apt failed.

Let me know if that workaround works.

This is a solvable growing pain, I hope to have all the Dorothy growing pains sorted in 2025H1, including a roll out of short videos showcasing everything Dorothy can do.

balupton added a commit that referenced this issue Dec 30, 2024
@edmcman
Copy link
Author

edmcman commented Dec 30, 2024

That worked, thanks!

@balupton
Copy link
Member

That worked, thanks!

Phew!

I've also updated the readme with a new dependency and requisites section:

@balupton balupton changed the title Install experience: Very slow, with long and large undesired dependency builds, and eventually fails Bug: Dorothy installation can fail if GitHub API Rate Limit is encountered again after the wait period Dec 31, 2024
@balupton balupton added the bug Broken or unexpected label Dec 31, 2024
@balupton balupton self-assigned this Dec 31, 2024
@balupton
Copy link
Member

balupton commented Jan 3, 2025

/ref #225

@balupton balupton added this to the Share Launch [v2] milestone Jan 3, 2025
@balupton
Copy link
Member

balupton commented Jan 6, 2025

The solution to this is multifold.

Todos:

  • different levels of caching within Dorothy of release asset api responses, such that rate limit is not hit in the first place; caching where request is not attempted, and caching where request is attempted but used if failed
  • have setup-util hit a Dorothy cloud which caches the asset api responses, the actual asset files, the prebuilt binaries from the recompiled assets, and the built binaries from source assets
  • have the Dorothy installation prompt for logging in with the github api
  • have rate limit encounters prompt for logging in with the github api, as an alternative to waiting
  • support recurrent wait or login prompts, instead of just one
  • fetch the api token from github cli or other tooling if they are logged in

@balupton
Copy link
Member

balupton commented Feb 4, 2025

I encountered the rate limit hit while debugging installations on fresh macos. However the wait worked fine for me, with no other machines interacting with Dorothy or github. gh was not logged in.

I will implement caching inside github-download which will be opt out, and will be cleared in setup-system clean.

@balupton
Copy link
Member

balupton commented Feb 9, 2025

Two improvements for this is coming in #281

There first is that we will now skip querying the GitHub repositories of utilities in not just quiet mode. Before it was constrained to only quiet mode. However other improvements since the original code for that, and some new modifications has enabled this change. This should assist in preventing the rate limit from being hit in the first place:

The second is that the Dorothy install experience will now be more suggestive of installing and logging in with the GitHub CLI, such that Dorothy can utilise its authentication token for hitting the API.

These two improvements should remove the need for a caching a solution. So I will wait and see after their release to note the impact and decide if any further improvements are justified.

balupton added a commit that referenced this issue Feb 9, 2025
cspell:
- update spellcheck for new utilities

command-working:
- add `ssh` to exceptions, this was needed as `setup-util-ssh` now checks for `ssh` as it handles the client too, which does a `ssh` check

cpr, fs-diff:
- update and correct comments on tooling and installers

dorothy:
- completely rewrote the user configuration setup process to reduce friction, streamline UX for beginners and pros, and eliminate fragility
- questions now have descriptions for YOLO no-RTFM users who install first and expect the installation to walk them through the project, shoutout to our YOLO user `@christianaudigeur6177` on youtube
- incentives authentication with github and gitlab to offer a superior repo creation/verification experience, and to prevent rate limiting /close #225 /improves #270
- able to now detect renamed github repositories if using the github cli, and prompts the user for what to do /ref #225
- robust and intuitive fallbacks if any step fails
- clear and robust strongbox setup, with improved description and flow, no longer waits for confirm as we now auto-verify
- strongbox and dorothy repo detection is now offloaded to `git-helper` rather than fetch, this means it now works on all repositories not just public github repositories, with none of the fragility, /close #266
- remove a legacy `__require_array 'mapfile'` /ref c158b97

get-installer:
- update aliases for the updated `setup-util-*` comments

git-helper:
- add `get-remote-file`, `has-remote-file`, `is-dorothy`, `is-strongbox` actions
- correct `--` usage in help text
- remote selection skips confirmation if there is only one remote

add:
- `setup-util-croc` intended for `cpr` and dorothy strongbox init
- `setup-util-diff` extracted from `setup-util-git`
- `setup-util-diff-so-fancy` intended for `fs-diff`
- `setup-util-difftastic` intended for `fs-diff`
- `setup-util-git-lfs` extracted from `setup-util-git`
- `setup-util-gitell` extracted from `setup-util-git`
- `setup-util-magic-wormwhole` intended for `cpr` and dorothy strongbox init
- `setup-util-meld` extracted from `setup-util-git`
- `setup-util-pinentry` extracted from `setup-util-git`
- `setup-util-ssh` handles the client and server, previously `setup-util-sshd` only handled server, that is not deprecated
- `setup-util-ssh-askpass` extracted from `setup-util-git`
- `setup-util-termscp` intended for `cpr` and dorothy strongbox init

setup-util:
- check is no longer constrained to `--quiet` mode, as we can just output the necessary text, this dramtically improves the successful checks which prevents DOWNLOAD determinations hitting the GitHub API causing Rate Limit encounters during the intial non-quiet dorothy setup experience /improves #270

setup-util-*:
- changed `setup-util "$@" --check --cli=<cli>` to `setup-util --check --cli=<cli> "$@"`, such that `--no-check` can be passed by the user to disable the checks

setup-util-gh:
- clearer messaging on why one would want to authenticate

setup-util-git:
- extract extra installers into their own `setup-util-*` scripts

setup-util-node:
- correct a syntax for a `git-helper` call

docs/private-configuration:
- update for new strongbox flow

deprecated:
- `setup-util-sshd` has been replaced by `setup-util-ssh` which now handles both client and server
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Broken or unexpected
Development

No branches or pull requests

2 participants