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

Downloading the binaries through a proxy #49

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ulrichard
Copy link
Contributor

Downloading the binaries through a proxy if either HTTPS_PROXY or HTTP_PROXY environment variable is set.

closes #48

Cargo.toml Outdated
@@ -18,7 +18,7 @@ home = "0.5.3" # use same ver in build-dep
env_logger = "0.8"

[build-dependencies]
ureq = "1.0" # allows to keep MSRV 1.41.1
ureq = "2.1"
Copy link
Contributor Author

@ulrichard ulrichard Jan 26, 2022

Choose a reason for hiding this comment

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

I hope it's ok to use the same version as electrsd. With this old version, the proxying didn't work, or at least not the same way as in electrsd.

Copy link
Collaborator

Choose a reason for hiding this comment

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

sorry for not giving an update on this but I prefer to put this in standby at the moment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, no hurry. I am blocked by #53 anyway at the moment. I had it running with #49 when we used Debian, but then we switched to Alpine in the Docker and it broke.

@ulrichard ulrichard force-pushed the feature/proxy branch 2 times, most recently from 93df4cd to 6e8ba5e Compare June 20, 2022 08:53
@ulrichard ulrichard force-pushed the feature/proxy branch 2 times, most recently from 09fb8d2 to 5c83cc7 Compare September 29, 2022 08:59
@ulrichard ulrichard force-pushed the feature/proxy branch 2 times, most recently from f57ec64 to 8b0fc8e Compare December 28, 2022 06:27
@RCasatta
Copy link
Collaborator

After #102 I would be inclined to accept this if you want to fix conflicts

@ulrichard
Copy link
Contributor Author

ok, rebased.
In the meantime I changed the strategy with our CI a bit. Now RCasatta/electrsd#51 would be more important to me, but it would still be good to have the proxy stuff included.

@ulrichard
Copy link
Contributor Author

I don't think the CI build error in 1.41.1 has anything to do with my changes.

Copy link
Collaborator

@RCasatta RCasatta left a comment

Choose a reason for hiding this comment

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

I don't think the CI build error in 1.41.1 has anything to do with my changes.

It doesn't which dep broke msrv with their new 4.4.0 version... Don't know what is best to do if pinning the dep or including a Cargo.lock, @Kixunil what do you think?

UPDATE: I'll go with this suggestion instead #104 (comment)

In the meantime I changed the strategy with our CI a bit. Now RCasatta/electrsd#51 would be more important to me,

I'll do my best to tackle that one too

@@ -114,10 +114,19 @@ mod download {
);
println!("url:{}", url);
let mut downloaded_bytes = Vec::new();
let resp = ureq::get(&url).call().unwrap();
assert_eq!(resp.status(), 200, "url {} didn't return 200", url);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't remember exactly why this assert is here but maybe it helps with reporting? Can we restore it?

- uses: actions-rs/cargo@v1
with:
command: update
args: -p which --precise 4.3.0
Copy link

Choose a reason for hiding this comment

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

Yeah, this is one of two good ways I know of to resolve the MSRV problem.

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.

Download using http proxy
3 participants