-
-
Notifications
You must be signed in to change notification settings - Fork 402
Fix upload of unchanged packages in S3 on source update of published repository #1440
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
Fix upload of unchanged packages in S3 on source update of published repository #1440
Conversation
Signed-off-by: Christoph Fiehe <[email protected]>
41ffe14 to
67bd154
Compare
d54a1df to
9b716d4
Compare
9b716d4 to
9ef217b
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1440 +/- ##
=======================================
Coverage 75.03% 75.03%
=======================================
Files 158 158
Lines 18356 18356
=======================================
Hits 13773 13773
Misses 3445 3445
Partials 1138 1138 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
and fix warnings.
3ad5d24 to
e062df6
Compare
|
pipeline fixed. |
d4fc670 to
bf5e700
Compare
bf5e700 to
e447fc0
Compare
|
Articafts for testing can be found here: https://github.com/aptly-dev/aptly/actions/runs/14571537492 |
|
@cfiehe I wonder if we can somehow test this behavioe in a system test. maybe logging all uploads in debug mode, and the test would check the output ? |
The question is: What should be covered by the test case? It would be the best, if you can somehow check, that only the expected files are uploaded to the S3 storage and validate afterwards that the files are present in the S3. I must admit, that I have not looked at the existing S3 test cases. |
|
I managed to get this fix tested with the existing s3 tests, by adding
With your fix reverted, the tests fail because the already uploaded package above is uploaded again. will cleanup and integrate here... |
* initialize zerolog for commands * Change default log format: remote colors and timestamp
sort both aptly output and gold file. output original output for debugging on failure. * Makefile: enable CAPTURE=1 env variable for capturing gold files * docker-system-test: use AWS env vars for S3 tests * fix system tests timing issue with order of gpg logs in publish tests
Ah, cool. That sounds good 👍. |
Fixes #1435.
Description of the Change
This PR fixes an issue in case of S3 storage that causes unchanged packages to be uploaded again on a source update of a published repository. The
storage.prefixwas added twice and the return paths from the storage did not contain thepooldirectory as first segment. That was the reason, why the code came to the conclusion, that the package does not exist in the bucket at the target location and uploaded the file again.