reown-com / a2

An Asynchronous Apple Push Notification (apns2) Client for Rust
MIT License
150 stars 52 forks source link

feat: Add option to set a request timeout #81

Closed threema-donat closed 6 months ago

threema-donat commented 6 months ago

Description

Adds the option to set a timeout on requests. For that, the existing options endpoint and signer are moved to a new ClientOptions struct that also allows setting the pool and request timeout (in seconds).

Resolves #73

How Has This Been Tested?

This has not yet been tested since my testing framework of choice (wiremock) does not support setting a timeout.

Due Dilligence

chris13524 commented 6 months ago

Please resolve conflicts :)

threema-donat commented 6 months ago

Some more suggestions on how to further refine these options into a full builder pattern:

* Rename `ClientOptions` to `ClientBuilder`

* Remove `endpoint` param from `ClientBuilder::new()` and default to `Production`

* Add `builder() -> ClientBuilder` method to `Client`

* Add `build(self)` method to `ClientBuilder` which will be the only mechanism to construct `Client`

* Remove params from `Client::new()` and implement with with `ClientBuilder::new().build()`. Delete `Client::new_with_defaults()`

Another example

What do you think?

I think it makes definitely sense there, but for passing the arguments from the user to the library I would suggest to have a simple struct since a builder for three fields is rather too complex. I added a ClientConfig::new convenience fn since most users probably want to configure this but not the other fields.