Skip to content

Change vanilla JAR download timeout and add warnings#474

Merged
Defman merged 1 commit into
feather-rs:mainfrom
Iaiao:download_timeout
Sep 18, 2021
Merged

Change vanilla JAR download timeout and add warnings#474
Defman merged 1 commit into
feather-rs:mainfrom
Iaiao:download_timeout

Conversation

@Iaiao

@Iaiao Iaiao commented Sep 18, 2021

Copy link
Copy Markdown
Contributor

Change vanilla JAR download timeout and add warnings

Status

  • Ready
  • Development
  • Hold

Description

Changed vanilla JAR download timeout, made it const
Added warnings when the network is very slow (with 2, 4, 6, ... minutes timeout). No one will see this because we don't have a logging frontend in feather-datapacks build script. Also, in build scripts, the stdout is only saved to target/debug/build/feather-datapacks-{hash}/output and not displayed on cargo build's stdout (except for cargo build -vv, which shows too much output to find the warning)

Related issues

#473

Checklist

  • Ran cargo fmt, cargo clippy --all-targets, cargo build --release and cargo test and fixed any generated errors!
  • Removed unnecessary commented out code
  • Used specific traces (if you trace actions please specify the cause i.e. the player)

Note: if you locally don't get any errors, but GitHub Actions fails (especially at clippy) you might want to check your rust toolchain version. You can then feel free to fix these warnings/errors in your PR.

@Defman

Defman commented Sep 18, 2021

Copy link
Copy Markdown
Member

Suggestion to switch to reqwest, which have better support for getting chunks. Then advancing the timer on every chunk instead of separate thread.

@Iaiao

Iaiao commented Sep 18, 2021

Copy link
Copy Markdown
Contributor Author

reqwest doesn't have chunks in reqwest::blocking, and I'm not sure it's ok to do something like tokio::spawn_blocking just for a simple warning. Also, the warning is for users with bad internet connection, so they may not receive chunks or receive them with a delay, but the warning should not be delayed because of these problems.

@Defman Defman merged commit b5ccddf into feather-rs:main Sep 18, 2021
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