XAMPPRocky / octocrab

A modern, extensible GitHub API Client for Rust.
Other
1.1k stars 265 forks source link

feat: allow sending non String payload with execute #665

Closed loispostula closed 3 months ago

loispostula commented 3 months ago

resolves #360

The approach I took is similar to the one outline here: https://github.com/XAMPPRocky/octocrab/issues/360#issuecomment-1547219725

Breaking changes

Drop of internal retry policy handler

Having to clone the request is a real challenge, as stated here: https://github.com/XAMPPRocky/octocrab/issues/360#issuecomment-1548524619 and https://github.com/hyperium/tonic/issues/733. I am in the opinion that such behaviour should not be inside of this library but in the client code, where they can use tools like tryhard to achieve reliable retry mechanism.

XAMPPRocky commented 3 months ago

Thank you for your PR!

I am in the opinion that such behaviour should not be inside of this library but in the client code, where they can use tools like tryhard to achieve reliable retry mechanism.

I do not agree with this opinion, the octocrab client should be able of handling retries and throttling, just like the existing octokit client. I'm not going to merge a solution that removes this functionality. It may be a challenge but this is functionality that is worth preserving. If you're willing to make changes to this PR to support cloning streaming bodies, I'd be happy to merge it.

loispostula commented 3 months ago

@XAMPPRocky I' ve updated the pr to allow cloning the request by changng OctoBody<BoxBody> to OctoBody<Arc<RwLock<BoxBody>>>

XAMPPRocky commented 3 months ago

Thank you for your PR, and congrats on your first contribution! 🎉