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

HTTP backend client cleanup #24639

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

Conversation

ash2k
Copy link
Contributor

@ash2k ash2k commented Apr 11, 2020

  1. Don't need to close response body twice.
  2. Pass data as []byte, no need to wrap in a reader. Wrapping causes an extra pointless copy.

@ash2k ash2k mentioned this pull request Apr 11, 2020
@ash2k
Copy link
Contributor Author

ash2k commented Jun 5, 2020

Hello there! Is something blocking this PR?

p.s. I'd like to add support for fetching HTTP backend configuration from environment variables. Would that be something that you would consider merging?

Base automatically changed from master to main February 24, 2021 18:01
@ash2k
Copy link
Contributor Author

ash2k commented Apr 22, 2021

Hello? =) 1 year has passed and this trivial PR still hasn't been reviewed.

1. Don't need to close response body twice
2. Pass data as []byte, no need to wrap in a reader
@hashicorp-cla
Copy link

hashicorp-cla commented Mar 12, 2022

CLA assistant check
All committers have signed the CLA.

@ash2k
Copy link
Contributor Author

ash2k commented Jun 1, 2022

2 years...

@crw
Copy link
Contributor

crw commented Jun 11, 2022

Thanks for this submission! The reason it has not been reviewed (at least recently) is due to the general hold on backend reviews (see CONTRIBUTING.md#state-storage-backends. I will update this PR if/when that is lifted.

I suspect it was not reviewed historically both for the above reason, but also due to fear of unintentionally breaking something for http backend users in a part of the code that is otherwise not actively under development / does not have core developer attention on it at the moment. This is just my gut feeling about this PR, but hopefully gives a little insight behind the curtain when looking at even small PRs such as this.

Thanks again for your submission!

@ash2k
Copy link
Contributor Author

ash2k commented Jun 11, 2022

Thanks for an update, appreciated.

@radditude radditude requested a review from a team as a code owner November 8, 2023 19:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants