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

set idle timeout for io client #35

Closed
wants to merge 2 commits into from

Conversation

abdelaziz-mahdy
Copy link
Contributor

@abdelaziz-mahdy abdelaziz-mahdy commented Nov 2, 2024

i saw this change was reverted, but when i looked through the code it passes the client settings in the request requestInternalGeneric of rhttp_client so it seems logical to me that this change should work (correct me if i am wrong, i dont have that much knowledge with rust)

related to #27

@Tienisto
Copy link
Owner

Tienisto commented Nov 2, 2024

copyWith is creating a new instance, so this line is a noop

@Tienisto
Copy link
Owner

Tienisto commented Nov 2, 2024

I've changed it into a noop, not throwing an exception anymore

@Tienisto Tienisto closed this Nov 2, 2024
@abdelaziz-mahdy
Copy link
Contributor Author

copyWith is creating a new instance, so this line is a noop

cant we just convert the type of the settings variable to non final and update it? that will make the set idle timeout work instead of being ignored? what do you think

@Tienisto
Copy link
Owner

Tienisto commented Nov 2, 2024

The settings is immutable because it is stored in Rust. If we make it mutable, then it would be a lot more complicated to keep everything in sync. Also, immutability is clean while Dart's mutable design is quick and dirty.

There is a workaround to make it work regardless.
We can store a mutable timeout class variable specifically for IoCompatibleClient and override the timeout for each request.
Such one-time override is currently not implemented though.

@abdelaziz-mahdy
Copy link
Contributor Author

Oh so the limitation is from the rust side

I thought the settings class gets sent with every request from the dart side

This is why I went with that approach 😅

Thanks for explaining

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