Add support for overriding github owned storage multipart upload chunk size via environment variable #1403
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…t variable Allows users to set an env variable GITHUB_OWNED_STORAGE_MULTIPART_BYTES with the number of bytes (min 5 Mib) for max number of bytes senter for each part in a multipart upload. By allowing the users to use a smaller size than the default, allows the upload to work in cases a user has a proxy that mishandles or blocks big uploads or for very slow connections. If not defined uses the default 100 Mib value
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for configuring the multipart upload chunk size for GitHub owned storage through an environment variable. The feature allows users to override the default 100 MiB chunk size with a custom value (minimum 5 MiB) by setting the GITHUB_OWNED_STORAGE_MULTIPART_BYTES environment variable.
- Added environment variable configuration for multipart upload chunk size with validation
- Implemented comprehensive test coverage for various edge cases and validation scenarios
- Updated release notes to document the new feature for users
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| RELEASENOTES.md | Added user-facing documentation for the new environment variable feature |
| src/Octoshift/Services/ArchiveUploader.cs | Implemented environment variable reading, validation, and chunk size configuration with minimum size enforcement |
| src/OctoshiftCLI.Tests/Octoshift/Services/ArchiveUploadersTests.cs | Added comprehensive unit tests for all validation scenarios and implemented proper test cleanup |
Comments suppressed due to low confidence (2)
src/OctoshiftCLI.Tests/Octoshift/Services/ArchiveUploadersTests.cs:62
- Accessing the private field
_streamSizeLimitdirectly in tests breaks encapsulation. Consider adding a public property or internal getter to expose this value for testing purposes.
archiveUploader._streamSizeLimit.Should().Be(customSize);
RELEASENOTES.md:1
- There are spelling and grammatical errors: 'a environment' should be 'an environment', 'you a proxy' should be 'you have a proxy', and 'Mib' should be 'MiB' for consistency with the code comments.
- Allow overriding the multipart upload chunk size when using GitHub owned storage. Set the value of the chunk (min 5Mib) in bytes in a environment variable called GITHUB_OWNED_STORAGE_MULTIPART_BYTES (default value 100Mib). Only set this if you a proxy that doesn't allow the default size upload or if you are on a very slow connection.
Unit Test Results 1 files 1 suites 21s ⏱️ Results for commit 688d690. ♻️ This comment has been updated with latest results. |
ArchiveUploader no longer reads the env variable directly. This eliminates the race condition in the tests
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This feature allows users to configure the multipart upload chunk size by setting the
GITHUB_OWNED_STORAGE_MULTIPART_BYTESenvironment variable.Configuration:
Use cases:
Checklist:
ThirdPartyNotices.txt(if applicable)closes #1402
Feature Enhancements:
GITHUB_OWNED_STORAGE_MULTIPART_BYTESenvironment variable, with a minimum of 5 MiB and a default of 100 MiB. This improves upload reliability in constrained network environments. (RELEASENOTES.md,src/Octoshift/Services/ArchiveUploader.cs,src/Octoshift/Services/EnvironmentVariableProvider.cs) [1] [2] [3]Code Refactoring:
Updated the
ArchiveUploaderconstructor to accept anEnvironmentVariableProviderinstance, enabling dynamic configuration of chunk sizes based on environment variables. (src/Octoshift/Services/ArchiveUploader.cs)Modified
GithubApiFactorymethods (Create,CreateClientNoSsl, andCreatefor target GitHub API) to passEnvironmentVariableProvidertoArchiveUploader. (src/Octoshift/Factories/GithubApiFactory.cs) [1] [2] [3]Test Updates:
Added extensive unit tests in
ArchiveUploaderTeststo validate behavior when the environment variable is set, not set, invalid, below minimum, zero, or negative. These tests ensure the default values and warnings are correctly applied. (src/OctoshiftCLI.Tests/Octoshift/Services/ArchiveUploadersTests.cs) [1] [2]Updated integration tests for
BbsToGithubandGhesToGithubto includeEnvironmentVariableProviderin theArchiveUploadersetup. (src/OctoshiftCLI.IntegrationTests/BbsToGithub.cs,src/OctoshiftCLI.IntegrationTests/GhesToGithub.cs) [1] [2]