CLI-1631: validating backup link before importing database #1958
Conversation
kalindi-adhiya
force-pushed
the
CLI-1631
branch
from
0ac894f to
37e199c
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1958 +/- ##
============================================
- Coverage 92.28% 92.28% -0.01%
- Complexity 1910 1918 +8
============================================
Files 122 122
Lines 6987 7008 +21
============================================
+ Hits 6448 6467 +19
- Misses 539 541 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Try the dev build for this PR: https://acquia-cli.s3.amazonaws.com/build/pr/1958/acli.phar |
There was a problem hiding this comment.
Pull request overview
This PR adds validation for database backup downloads to ensure files are valid before attempting to import them. The changes detect common failure scenarios (empty files, invalid gzip content, HTTP errors, missing files) and provide clear error messages.
Changes:
- Added validation methods to check HTTP response status, file existence, file size, and gzip format validity
- Updated test infrastructure to support validation error scenarios across Windows and non-Windows platforms
- Added comprehensive test coverage for empty files, invalid gzip content, missing files, small files, and HTTP errors
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/Command/Pull/PullCommandBase.php | Implements validation logic for backup downloads including HTTP response validation and gzip file format verification |
| tests/phpunit/src/Commands/Pull/PullDatabaseCommandTest.php | Adds test cases for various validation error scenarios and getter/setter tests for backup download URL |
| tests/phpunit/src/Commands/Pull/PullCommandTestBase.php | Updates mock infrastructure to support validation testing and handle Windows-specific filename generation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| $dbMachineName, | ||
| $backupResponse->completedAt, | ||
| ]) . '.sql.gz'; | ||
| if (PHP_OS_FAMILY === 'Windows') { |
There was a problem hiding this comment.
Why change this? I don't see anything in the parent ticket mentioning Windows compatibility
There was a problem hiding this comment.
@danepowell it was due to failing testcases specifically for windows
There was a problem hiding this comment.
I don't think that was meant for me, but I'm glad you found the problem!
kalindi-adhiya
force-pushed
the
CLI-1631
branch
from
c4c911c to
c6ab7ff
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
4 participants
Motivation
Fixes #NNN
Proposed changes
Alternatives considered
Testing steps
./bin/acli ckc