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

Make max retries configurable #33

Merged
merged 5 commits into from
Aug 4, 2024
Merged

Conversation

nick42d
Copy link
Contributor

@nick42d nick42d commented Aug 4, 2024

Perhaps more unlikely with the new changes from 3.8.24, but it could be still possible for chunk max retries to be insufficient. This PR allows library consumers to override the default number of retries. Used an Option in RequestOptions to work in with the derive(Default).

IMO - resolves #32.

@Mithronn
Copy link
Owner

Mithronn commented Aug 4, 2024

    /// Number of retries to allow per web request (ie, per chunk downloaded)
    /// Default is [`crate::constants::DEFAULT_RETRIES`].
    ///
    /// # Example
    /// 
    ///     // Allow 5 retries per chunk.
    ///     let video_options = VideoOptions {
    ///          request_options: RequestOptions {
    ///               max_retries: Some(5),
    ///                ..Default::default()
    ///          },
    ///          ..Default::default()
    ///     };
    /// 
    pub max_retries: Option<u32>,

max_retries more suitable for the option name

@nick42d
Copy link
Contributor Author

nick42d commented Aug 4, 2024

    /// Number of retries to allow per web request (ie, per chunk downloaded)
    /// Default is [`crate::constants::DEFAULT_RETRIES`].
    ///
    /// # Example
    /// 
    ///     // Allow 5 retries per chunk.
    ///     let video_options = VideoOptions {
    ///          request_options: RequestOptions {
    ///               max_retries: Some(5),
    ///                ..Default::default()
    ///          },
    ///          ..Default::default()
    ///     };
    /// 
    pub max_retries: Option<u32>,

max_retries more suitable for the option name

Updated now!

@Mithronn Mithronn merged commit dd8beae into Mithronn:main Aug 4, 2024
1 check passed
@nick42d nick42d deleted the non-vendored branch August 5, 2024 12:27
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.

Feature request: allow retries for chunks
2 participants