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.

Sign up for GitHub

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

Jump to bottom

Allow to create HTTP Sender with custom Client #33580

Merged
merged 2 commits into from
Oct 10, 2023
Merged

Allow to create HTTP Sender with custom Client #33580

merged 2 commits into from
Oct 10, 2023

Conversation

fanatid
Copy link
Contributor

@fanatid fanatid commented Oct 7, 2023

Problem

In Triton sometimes we use custom headers in RPC Client but it's hard to create HttpSender outside of solana crate so we need to use a slightly patched version. With HttpSender::new_with_client it would be possible to create a sender with completely controlled created reqwest::Client.

Summary of Changes

  • Add HttpSender::new_with_client
  • Move default headers to HttpSender::default_headers

@mergify mergify bot added community Community contribution need:merge-assist labels Oct 7, 2023
@mergify mergify bot requested a review from a team October 7, 2023 08:08
CriesofCarrots
CriesofCarrots reviewed Oct 10, 2023
View reviewed changes
Copy link
Contributor

@CriesofCarrots CriesofCarrots left a comment

Choose a reason for hiding this comment

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

Doc edits; the new code looks fine

rpc-client/src/http_sender.rs Outdated Show resolved Hide resolved
@CriesofCarrots CriesofCarrots added the CI Pull Request is ready to enter CI label Oct 10, 2023
@solana-grimes solana-grimes removed the CI Pull Request is ready to enter CI label Oct 10, 2023
@CriesofCarrots CriesofCarrots added the automerge Merge this Pull Request automatically once CI passes label Oct 10, 2023
@mergify mergify bot removed the automerge Merge this Pull Request automatically once CI passes label Oct 10, 2023
@mergify
Copy link
Contributor

mergify bot commented Oct 10, 2023

automerge label removed due to a CI failure

@fanatid
Copy link
Contributor Author

fanatid commented Oct 10, 2023

looks like unrelated test fail

@CriesofCarrots
Copy link
Contributor

looks like unrelated test fail

Retry to victory 😅

@CriesofCarrots CriesofCarrots added the automerge Merge this Pull Request automatically once CI passes label Oct 10, 2023
@codecov
Copy link

codecov bot commented Oct 10, 2023

Codecov Report

Merging #33580 (90273e7) into master (95810d8) will decrease coverage by 0.1%.
Report is 25 commits behind head on master.
The diff coverage is 100.0%.

@@            Coverage Diff            @@
##           master   #33580     +/-   ##
=========================================
- Coverage    81.7%    81.7%   -0.1%     
=========================================
  Files         807      807             
  Lines      218306   218313      +7     
=========================================
- Hits       178518   178511      -7     
- Misses      39788    39802     +14

@CriesofCarrots CriesofCarrots merged commit a226783 into solana-labs:master Oct 10, 2023
16 checks passed
@fanatid fanatid deleted the http-sender-custom-client branch October 10, 2023 20:43
@fanatid
Copy link
Contributor Author

fanatid commented Oct 10, 2023

Thanks @CriesofCarrots!

@fanatid
Copy link
Contributor Author

fanatid commented Oct 11, 2023

Can you please add v1.16 & v1.17 labels for backport? Thanks!

@CriesofCarrots CriesofCarrots added the v1.17 PRs that should be backported to v1.17 label Oct 11, 2023
mergify bot pushed a commit that referenced this pull request Oct 11, 2023
@fanatid @mergify
Allow to create HTTP Sender with custom Client (#33580)
3d25240 
* Allow to create HTTP Sender with custom Client

* Update rpc-client/src/http_sender.rs

Co-authored-by: Tyera <[email protected]>

---------

Co-authored-by: Tyera <[email protected]>
(cherry picked from commit a226783)
@mergify mergify bot mentioned this pull request Oct 11, 2023
Merged
@CriesofCarrots
Copy link
Contributor

Can you please add v1.16 & v1.17 labels for backport? Thanks!

v1.17 is still accepting new features/changes, but I don't think I can justify the v1.16 backport. We're working to speed up our minor-version release cadence, and part of that is being more rigorous at limiting stable-branch backports to critical bug fixes only.
I do get the argument why this would be fine for stable. We're discussing adding a description of our backport policy to our contributor docs; maybe when we do, we will distinguish between validator and non-validator features, and can reconsider this one.

mergify bot added a commit that referenced this pull request Oct 11, 2023
@mergify @fanatid
v1.17: Allow to create HTTP Sender with custom Client (backport of #3…
0e2bded 
…3580) (#33660)

Allow to create HTTP Sender with custom Client (#33580)

* Allow to create HTTP Sender with custom Client

* Update rpc-client/src/http_sender.rs

Co-authored-by: Tyera <[email protected]>

---------

Co-authored-by: Tyera <[email protected]>
(cherry picked from commit a226783)

Co-authored-by: Kirill Fomichev <[email protected]>
@HaoranYi HaoranYi mentioned this pull request Apr 8, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CriesofCarrots CriesofCarrots CriesofCarrots approved these changes

Assignees
No one assigned
Labels
automerge Merge this Pull Request automatically once CI passes community Community contribution need:merge-assist v1.17 PRs that should be backported to v1.17
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@fanatid @CriesofCarrots @solana-grimes