Use ClientOptions to create Pipeline #288
Conversation
sdk/core/src/options.rs
Outdated
| #[derive(Clone, Debug)] | ||
| pub struct RetryOptions { | ||
| /// The algorithm to use for calculating retry delays. | ||
| pub(crate) mode: RetryMode, |
There was a problem hiding this comment.
nit: if we have setters already is there a need for the pub(crate) annotation on the fields?
There was a problem hiding this comment.
I need to access these in scopes that are children of a sibling scope.
There was a problem hiding this comment.
I am not sure I understand why. I checkout out this branch, removed the pub(crate) and it compiles.
There was a problem hiding this comment.
I'll try it. The docs said (paraphrasing - or perhaps I misunderstood entirely) that pub(crate) was necessary to access members of sibling mods, or children of sibling mods.
There was a problem hiding this comment.
I was able to remove them from some of the members but not all. Started with removing them all and added them back in as needed.
|
Oh, and I forgot to add some more examples like you mentioned in your overarching comment. I'll do that this weekend before merging if you feel appropriate to change your vote. |
|
I opened #294 to track moving (exposing) request methods from the pipeline itself, like with our other SDKs, to abstract away the requests from the wire protocol. |
sdk/core/src/options.rs
Outdated
| #[derive(Clone, Debug)] | ||
| pub struct RetryOptions { | ||
| /// The algorithm to use for calculating retry delays. | ||
| pub(crate) mode: RetryMode, |
There was a problem hiding this comment.
I am not sure I understand why. I checkout out this branch, removed the pub(crate) and it compiles.
|
I opened #300 to further the discussion on how we want the guidelines to end up in this regard. |
|
|
||
| impl ClientOptions { | ||
| /// A mutable reference to per-call policies. | ||
| pub fn mut_per_call_policies(&mut self) -> &mut Vec<Arc<dyn Policy>> { |
There was a problem hiding this comment.
The idiomatic form is per_call_policies_mut
| } | ||
|
|
||
| /// A mutable reference to per-retry policies. | ||
| pub fn mut_per_retry_policies(&mut self) -> &mut Vec<Arc<dyn Policy>> { |
|
|
||
| /// Gets the `HttpClient` used by the pipeline. | ||
| pub fn http_client(&self) -> &dyn HttpClient { | ||
| // TODO: Request methods should be defined directly on the pipeline instead of exposing the HttpClient. |
There was a problem hiding this comment.
Can you create an issue for this?
There was a problem hiding this comment.
Ah I see that there's this: #294. Might be worth in the future having all todos link to an issue
| max_delay: Duration::from_secs(10), | ||
| impl FixedRetryPolicy { | ||
| pub(crate) fn new(delay: Duration, max_retries: u32, max_delay: Duration) -> Self { | ||
| FixedRetryPolicy { |
There was a problem hiding this comment.
Why the change away from Self?
No description provided.