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

Adds overloads to GetArchive for adding custom timeouts #918

Merged
merged 3 commits into from
Nov 4, 2015
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions Octokit.Reactive/Clients/IObservableRepositoryContentsClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,30 @@ public interface IObservableRepositoryContentsClient
/// <returns>A promise, containing the binary contents of the archive</returns>
IObservable<byte[]> GetArchive(string owner, string name, ArchiveFormat archiveFormat, string reference);

/// <summary>
/// Get an archive of a given repository's contents, in a specific format
/// </summary>
/// <remarks>https://developer.github.com/v3/repos/contents/#get-archive-link</remarks>
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <param name="archiveFormat">The format of the archive. Can be either tarball or zipball</param>
/// <param name="reference">A valid Git reference.</param>
/// <param name="timeout"> Timeout in minutes </param>
/// <returns>The binary contents of the archive</returns>
IObservable<byte[]> GetArchive(string owner, string name, ArchiveFormat archiveFormat, string reference, int timeout);
Copy link
Contributor

Choose a reason for hiding this comment

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

Honestly, I'd rather not have this method and only have the TimeSpan I think it promotes better practices. Also, if we really want this overload, we can add it as an extension method to the interface.


/// <summary>
/// Get an archive of a given repository's contents, in a specific format
/// </summary>
/// <remarks>https://developer.github.com/v3/repos/contents/#get-archive-link</remarks>
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <param name="archiveFormat">The format of the archive. Can be either tarball or zipball</param>
/// <param name="reference">A valid Git reference.</param>
/// <param name="timeout"> Time span until timeout </param>
/// <returns>The binary contents of the archive</returns>
IObservable<byte[]> GetArchive(string owner, string name, ArchiveFormat archiveFormat, string reference, TimeSpan timeout);

/// <summary>
/// Returns the contents of a file or directory in a repository.
/// </summary>
Expand Down
30 changes: 30 additions & 0 deletions Octokit.Reactive/Clients/ObservableRepositoryContentsClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,36 @@ public IObservable<RepositoryContent> GetAllContents(string owner, string name,
.GetAndFlattenAllPages<RepositoryContent>(ApiUrls.RepositoryContent(owner, name, path));
}

/// <summary>
/// Get an archive of a given repository's contents, in a specific format
/// </summary>
/// <remarks>https://developer.github.com/v3/repos/contents/#get-archive-link</remarks>
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <param name="archiveFormat">The format of the archive. Can be either tarball or zipball</param>
/// <param name="reference">A valid Git reference.</param>
/// <param name="timeout"> Timeout in minutes </param>
/// <returns>The binary contents of the archive</returns>
public IObservable<byte[]> GetArchive(string owner, string name, ArchiveFormat archiveFormat, string reference, int timeout)
{
return _client.Repository.Content.GetArchive(owner, name, archiveFormat, reference, timeout).ToObservable();
}

/// <summary>
/// Get an archive of a given repository's contents, in a specific format
/// </summary>
/// <remarks>https://developer.github.com/v3/repos/contents/#get-archive-link</remarks>
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <param name="archiveFormat">The format of the archive. Can be either tarball or zipball</param>
/// <param name="reference">A valid Git reference.</param>
/// <param name="timeout"> Time span until timeout </param>
/// <returns>The binary contents of the archive</returns>
public IObservable<byte[]> GetArchive(string owner, string name, ArchiveFormat archiveFormat, string reference, TimeSpan timeout)
{
Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably validate that the TimeSpan is not negative as a negative value is almost certainly a programmer error. I'd throw an ArgumentOutOfRange exception.

Copy link
Member

Choose a reason for hiding this comment

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

👍 done on line 182

return _client.Repository.Content.GetArchive(owner, name, archiveFormat, reference, timeout).ToObservable();
}

/// <summary>
/// Returns the contents of a file or directory in a repository.
/// </summary>
Expand Down
24 changes: 24 additions & 0 deletions Octokit/Clients/IRepositoryContentsClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,30 @@ public interface IRepositoryContentsClient
/// <returns>The binary contents of the archive</returns>
Task<byte[]> GetArchive(string owner, string name, ArchiveFormat archiveFormat, string reference);

/// <summary>
/// Get an archive of a given repository's contents, in a specific format
/// </summary>
/// <remarks>https://developer.github.com/v3/repos/contents/#get-archive-link</remarks>
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <param name="archiveFormat">The format of the archive. Can be either tarball or zipball</param>
/// <param name="reference">A valid Git reference.</param>
/// <param name="timeout"> Timeout in minutes </param>
/// <returns>The binary contents of the archive</returns>
Task<byte[]> GetArchive(string owner, string name, ArchiveFormat archiveFormat, string reference, int timeout);

/// <summary>
/// Get an archive of a given repository's contents, in a specific format
/// </summary>
/// <remarks>https://developer.github.com/v3/repos/contents/#get-archive-link</remarks>
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <param name="archiveFormat">The format of the archive. Can be either tarball or zipball</param>
/// <param name="reference">A valid Git reference.</param>
/// <param name="timeout"> Time span until timeout </param>
/// <returns>The binary contents of the archive</returns>
Task<byte[]> GetArchive(string owner, string name, ArchiveFormat archiveFormat, string reference, TimeSpan timeout);

/// <summary>
/// Creates a commit that creates a new file in a repository.
/// </summary>
Expand Down
34 changes: 32 additions & 2 deletions Octokit/Clients/RepositoryContentsClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -187,14 +187,44 @@ public Task<string> GetArchiveLink(string owner, string name, ArchiveFormat arch
/// <param name="archiveFormat">The format of the archive. Can be either tarball or zipball</param>
/// <param name="reference">A valid Git reference.</param>
/// <returns>The binary contents of the archive</returns>
public async Task<byte[]> GetArchive(string owner, string name, ArchiveFormat archiveFormat, string reference)
public Task<byte[]> GetArchive(string owner, string name, ArchiveFormat archiveFormat, string reference)
{
return GetArchive(owner, name, archiveFormat, string.Empty, 60);
}

/// <summary>
/// Get an archive of a given repository's contents, in a specific format
/// </summary>
/// <remarks>https://developer.github.com/v3/repos/contents/#get-archive-link</remarks>
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <param name="archiveFormat">The format of the archive. Can be either tarball or zipball</param>
/// <param name="reference">A valid Git reference.</param>
/// <param name="timeout"> Timeout in minutes </param>
/// <returns>The binary contents of the archive</returns>
public Task<byte[]> GetArchive(string owner, string name, ArchiveFormat archiveFormat, string reference, int timeout)
{
return GetArchive(owner, name, archiveFormat, string.Empty, TimeSpan.FromMinutes(60));
}

/// <summary>
/// Get an archive of a given repository's contents, in a specific format
/// </summary>
/// <remarks>https://developer.github.com/v3/repos/contents/#get-archive-link</remarks>
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <param name="archiveFormat">The format of the archive. Can be either tarball or zipball</param>
/// <param name="reference">A valid Git reference.</param>
/// <param name="timeout"> Time span until timeout </param>
/// <returns>The binary contents of the archive</returns>
public async Task<byte[]> GetArchive(string owner, string name, ArchiveFormat archiveFormat, string reference, TimeSpan timeout)
{
Ensure.ArgumentNotNullOrEmptyString(owner, "owner");
Ensure.ArgumentNotNullOrEmptyString(name, "name");

Copy link
Contributor

Choose a reason for hiding this comment

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

Validate the TimeSpan argument here too please.

var endpoint = ApiUrls.RepositoryArchiveLink(owner, name, archiveFormat, reference);

var response = await Connection.Get<byte[]>(endpoint, TimeSpan.FromMinutes(60));
var response = await Connection.Get<byte[]>(endpoint, timeout);

return response.Body;
}
Expand Down