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

Add cancellation token support for release assets uploading #2267

Merged
merged 4 commits into from Feb 21, 2021

Conversation

KatoStoelen
Copy link
Contributor

Small spike for #1840

Added an optional cancellation token to IReleasesClient.UploadAsset(...).

To make it easier to continue this work, I did the same for all the Post methods in IApiConnection and IConnection as well.

Comment on lines +512 to +524
private class CancellationTestHttpClient : IHttpClient
{
public async Task<IResponse> Send(IRequest request, CancellationToken cancellationToken)
{
await Task.Delay(TimeSpan.FromSeconds(1), cancellationToken);

throw new Exception("HTTP operation was not cancelled");
}

public void Dispose() { }

public void SetRequestTimeout(TimeSpan timeout) { }
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Couldn't figure out how to set this up using NSubstitute. Hence, this mock of IHttpClient.

Copy link
Member

Choose a reason for hiding this comment

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

I think this is fine 👍🏻

Choose a reason for hiding this comment

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

fuckd up

  • [ ]

@codecov
Copy link

codecov bot commented Nov 2, 2020

Codecov Report

Merging #2267 (6ec6df3) into main (4ca8ea0) will increase coverage by 0.02%.
The diff coverage is 50.00%.

@@            Coverage Diff             @@
##             main    #2267      +/-   ##
==========================================
+ Coverage   65.97%   66.00%   +0.02%     
==========================================
  Files         553      553              
  Lines       14435    14437       +2     
  Branches      844      844              
==========================================
+ Hits         9524     9529       +5     
+ Misses       4752     4749       -3     
  Partials      159      159              
Impacted Files Coverage Δ
Octokit/Http/Connection.cs 63.72% <25.00%> (+1.01%) ⬆️
Octokit/Http/ApiConnection.cs 60.25% <42.85%> (ø)
...tokit.Reactive/Clients/ObservableReleasesClient.cs 100.00% <100.00%> (ø)
Octokit/Clients/ReleasesClient.cs 100.00% <100.00%> (ø)

@shiftkey shiftkey changed the title Cancellation spike - release assets uploading Add cancellation token support for release assets uploading Feb 13, 2021
As this is only a spike for octokit#1840, only the POST methods are targeted
for now.
As this is only a spike for octokit#1840, only the POST methods are targeted
for now.
Spike of octokit#1840 to enable cancellation of release asset uploading.
@shiftkey shiftkey force-pushed the cancellation-spike-release-assets branch from 2ad2839 to 093a30c Compare February 13, 2021 20:24
Copy link
Member

@shiftkey shiftkey left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution @KatoStoelen!

@shiftkey shiftkey merged commit 4fbbe4c into octokit:main Feb 21, 2021
@shiftkey
Copy link
Member

release_notes: Added cancellation token support for release assets uploading API

@nickfloyd nickfloyd added Type: Feature New feature or request and removed category: feature labels Oct 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants