Skip to content

Rename streamablehttp_client to streamable_http_client #1177

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Kludex
Copy link
Member

@Kludex Kludex commented Jul 21, 2025

The current name is not very intuitive considering the class name and the module.

@Kludex Kludex requested review from a team as code owners July 21, 2025 08:43
@Kludex Kludex requested a review from ochafik July 21, 2025 08:43
@Kludex Kludex requested a review from ihrpr July 21, 2025 08:44
Copy link
Contributor

@ihrpr ihrpr left a comment

Choose a reason for hiding this comment

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

The deprecated wrapper is missing the httpx_client_factory and auth parameters. This could break existing code using those parameters.

(it's probably a good time to replace httpx_client_factory with an object)

@Kludex
Copy link
Member Author

Kludex commented Jul 21, 2025

I blame copilot. 😄

@Kludex
Copy link
Member Author

Kludex commented Jul 21, 2025

(it's probably a good time to replace httpx_client_factory with an object)

I think the timeouts, the auth and the httpx_client_factory CAN be replaced. The timeouts I think can be subjective, but the auth and the httpx_client_factory I think need to be replaced.

@ihrpr
Copy link
Contributor

ihrpr commented Jul 21, 2025

(it's probably a good time to replace httpx_client_factory with an object)

I think the timeouts, the auth and the httpx_client_factory CAN be replaced. The timeouts I think can be subjective, but the auth and the httpx_client_factory I think need to be replaced.

should we just replace it now, since we are deprecating one method, would be good to have a nice alternative and not to deprecate it twice?

@Kludex
Copy link
Member Author

Kludex commented Jul 21, 2025

(it's probably a good time to replace httpx_client_factory with an object)

I think the timeouts, the auth and the httpx_client_factory CAN be replaced. The timeouts I think can be subjective, but the auth and the httpx_client_factory I think need to be replaced.

should we just replace it now, since we are deprecating one method, would be good to have a nice alternative and not to deprecate it twice?

I didn't get what you said. I agree that it's a good moment to change the parameters of the new streamable_http_client.

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