Pull ozu data from inside worker #23
Conversation
There was a problem hiding this comment.
Pull request overview
This PR enables pulling Ozu CMS data from inside worker processes by making the data fetch command more flexible and switching to a simpler image processing driver.
Key changes:
- Switched from Imagick to GD image driver for local thumbnails (worker-friendly, doesn't require ImageMagick extension)
- Removed environment-based logic for CDN selection, simplifying the provider selection
- Added
--forceand--withoutAssetsoptions to the fetch command for better automation support
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/Support/Thumbnails/LocalThumbnail.php |
Changed image driver from Imagick to GD for better compatibility in worker environments |
src/OzuServiceProvider.php |
Removed environment check from CDN provider selection logic and improved comments |
src/Console/FetchDataFromOzu.php |
Added command options for non-interactive execution (--force to skip confirmation, --withoutAssets to skip asset downloads) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
| class FetchDataFromOzu extends Command | ||
| { | ||
| protected $signature = 'ozu:fetch-ozu-data'; | ||
| protected $signature = 'ozu:fetch-ozu-data {--force : Do not ask for confirmation} {--withoutAssets : Do not download assets}'; |
There was a problem hiding this comment.
The option name withoutAssets should use kebab-case following Laravel conventions. It should be --without-assets instead of --withoutAssets. This improves consistency with standard Laravel command options.
| )) { | ||
| return self::SUCCESS; | ||
| $this->newLine(2); | ||
| $this->warn('⚠️ This action will erase your local database and assets.'); |
There was a problem hiding this comment.
Extra space in the warning emoji. Should be ⚠️ instead of ⚠️ (there are two spaces after the emoji).
| $this->warn('⚠️ This action will erase your local database and assets.'); | |
| $this->warn('⚠️ This action will erase your local database and assets.'); |
| class FetchDataFromOzu extends Command | ||
| { | ||
| protected $signature = 'ozu:fetch-ozu-data'; | ||
| protected $signature = 'ozu:fetch-ozu-data {--force : Do not ask for confirmation} {--withoutAssets : Do not download assets}'; |
There was a problem hiding this comment.
The new command options --force and --withoutAssets lack test coverage. Consider adding tests to verify:
- The
--forceoption bypasses the confirmation prompt - The
--withoutAssetsoption skips asset download and extraction - Both options work correctly together
Similar commands in this project have comprehensive test coverage (e.g., FetchSettingsFromOzu).
No description provided.