Skip to content

Conversation

toondaey
Copy link
Contributor

@toondaey toondaey commented Jul 30, 2023

Describe your proposed changes here.

This changes will allow users to configure backoff/retry to their liking. This is also one of a 2 part changes (other still in progress) in an attempt to also allow a form of termination and not try forever especially without informing the user of the progress.

  • I've read the contributing section of the project CONTRIBUTING.md.
  • Signed CLA (if not already signed).

src/backoff.rs Outdated
///
/// See <https://aws.amazon.com/blogs/architecture/exponential-backoff-and-jitter/>
#[derive(Debug, Clone)]
#[derive(Debug, Clone, Copy)]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would prefer to NOT have this Copy so we can extend this later. Copy should only be used for very small types (IIRC rustc triggers this up to 256 bytes).

If you have trouble w/ clippy/rustc complaining about a missing copy impl, stick an #[allow(missing_copy_implementations)] to this type.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure if this creates any other form of violation, however, I had to switch to using Arc for the BackoffConfig just for simplicity and also to avoid having to introduce complexity with using lifetimes.

@toondaey toondaey requested a review from crepererum July 31, 2023 18:55
@crepererum crepererum changed the title Clientbuilder accept backoffconfig refactor: Clientbuilder accept backoffconfig Aug 2, 2023
@crepererum
Copy link
Collaborator

Thank you.

@crepererum crepererum added the automerge Instruct kodiak to merge the PR label Aug 2, 2023
@kodiakhq kodiakhq bot merged commit 92e4cfe into influxdata:main Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Instruct kodiak to merge the PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants