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 support for storing the "local" pool on Azure #1283

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

neolynx
Copy link
Member

@neolynx neolynx commented Apr 21, 2024

Replaces #1074

Requirements

All new code should be covered with tests, documentation should be updated. CI should pass.

Description of the Change

Checklist

  • unit-test added (if change is algorithm)
  • functional test added/updated (if change is functional)
  • man page updated (if applicable)
  • bash completion updated (if applicable)
  • documentation updated
  • author name in AUTHORS

@neolynx neolynx self-assigned this Apr 21, 2024
@neolynx
Copy link
Member Author

neolynx commented Apr 21, 2024

might have rebase errors in azure/public.go since prefix was introduced

Copy link

codecov bot commented Apr 21, 2024

Codecov Report

Attention: Patch coverage is 67.30245% with 120 lines in your changes are missing coverage. Please review.

Project coverage is 74.67%. Comparing base (8d09c20) to head (7b1917f).
Report is 1 commits behind head on master.

Files Patch % Lines
azure/package_pool.go 64.33% 34 Missing and 17 partials ⚠️
azure/azure.go 76.82% 13 Missing and 6 partials ⚠️
api/mirror.go 25.00% 9 Missing and 3 partials ⚠️
cmd/mirror_update.go 25.00% 9 Missing and 3 partials ⚠️
utils/config.go 62.50% 9 Missing and 3 partials ⚠️
files/public.go 47.61% 8 Missing and 3 partials ⚠️
context/context.go 83.33% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1283      +/-   ##
==========================================
- Coverage   74.79%   74.67%   -0.12%     
==========================================
  Files         144      146       +2     
  Lines       16256    16489     +233     
==========================================
+ Hits        12158    12313     +155     
- Misses       3156     3205      +49     
- Partials      942      971      +29     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@neolynx neolynx requested review from mfolusiak, szakalboss and a team April 21, 2024 12:34
@neolynx neolynx added the needs review Ready for review & merge label Apr 21, 2024
@neolynx neolynx added the increase coverage The PR lacks test coverage label Apr 21, 2024
refi64 and others added 12 commits April 24, 2024 17:39
read_path() can read in binary, which the S3 tests don't support (simply
because they don't need it)...but it needs to be able to take the `mode`
argument anyway.

Signed-off-by: Ryan Gonzalez <ryan.gonzalez@collabora.com>
Before, a "partial" URL (either "localhost:port" or an endpoint URL
*without* the account name as the subdomain) would be specified, and the
full one would automatically be inferred. Although this is somewhat
nice, it means that the endpoint string doesn't match the official Azure
syntax:

https://docs.microsoft.com/en-us/azure/storage/common/storage-configure-connection-string

This also raises issues for the creation of functional tests for Azure,
as the code to determine the endpoint string needs to be duplicated
there as well.

Instead, it's just easiest to follow Azure's own standard, and then
sidestep the need for any custom logic in the functional tests.

Signed-off-by: Ryan Gonzalez <ryan.gonzalez@collabora.com>
Signed-off-by: Ryan Gonzalez <ryan.gonzalez@collabora.com>
Signed-off-by: Ryan Gonzalez <ryan.gonzalez@collabora.com>
Signed-off-by: Ryan Gonzalez <ryan.gonzalez@collabora.com>
The contents of `os.Stat` are rather fitted towards local package pools,
but the method is in the generic PackagePool interface. This moves it to
LocalPackagePool, and the use case of simply finding a file's size is
delegated to a new, more generic PackagePool.Size() method.

Signed-off-by: Ryan Gonzalez <ryan.gonzalez@collabora.com>
Several sections of the code *required* a LocalPackagePool, but they
could still perform their operations with a standard PackagePool.

Signed-off-by: Ryan Gonzalez <ryan.gonzalez@collabora.com>
Signed-off-by: Ryan Gonzalez <ryan.gonzalez@collabora.com>
This adds support for storing packages directly on Azure, with no truly
"local" (on-disk) repo used. The existing Azure PublishedStorage
implementation was refactored to move the shared code to a separate
context struct, which can then be re-used by the new PackagePool. In
addition, the files package's mockChecksumStorage was made public so
that it could be used in the Azure PackagePool tests as well.

Signed-off-by: Ryan Gonzalez <ryan.gonzalez@collabora.com>
fixes golangci-lint errors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
increase coverage The PR lacks test coverage needs review Ready for review & merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants