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

[1.x] Build and pull images on install #467

Merged
merged 6 commits into from
Aug 16, 2022
Merged

[1.x] Build and pull images on install #467

merged 6 commits into from
Aug 16, 2022

Conversation

jessarcher
Copy link
Member

@jessarcher jessarcher commented Aug 16, 2022

This PR updates the sail:install command so that it will download and build the required Docker images upon installation.

When running sail up on a fresh machine, it can take more than 5 minutes to download and build everything before it transitions to the serving phase. If you're unfamiliar with Docker's output, it is not immediately apparent when the downloading and building have finished and the serving has begun.

While this won't decrease waiting times, it moves the "slow part" from the up command to the install command, where it's far more apparent when it has been completed. When running the up command, everything can start straight away.

Note that the database containers go through a brief "first run" setup phase where the initial user and databases are created. While users may immediately access their laravel.test container, they may briefly encounter a database connection error if they're quick enough to hit the database before it's ready.

I have also added some validation to the --with option to ensure that we only pass sanitized data to the sail pull shell command. In doing so, I noticed that we were not adding a "depends_on" entry for memcached, which I have also resolved.

Happy path

image

With Docker service stopped

image

(The output from the sail pull and sail build commands is passed through to help the developer debug why the install failed, as it could be various issues such as no network)

If this is merged, I will PR the docs to remove this section from https://laravel.com/docs/9.x#getting-started-on-macos, and maybe mention the time under the install command instead:

image

@jessarcher jessarcher marked this pull request as draft August 16, 2022 06:45
@jessarcher jessarcher marked this pull request as ready for review August 16, 2022 07:10
Comment on lines -80 to +97
->filter(function ($service) {
return in_array($service, ['mysql', 'pgsql', 'mariadb', 'redis', 'meilisearch', 'minio', 'selenium']);
})->map(function ($service) {
->map(function ($service) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was missing the memcached service, but now that we're validating the services, I don't think we need to filter it.

@jf-prevost
Copy link
Contributor

Hi, with Laravel framework v9.3.11 / Sail v1.16.2 , doing a "php artisan sail:install --with=none" ends up crashing because Sail tries to pull images from an empty service list.
Guess we should avoid pulling images if no additional service is needed (f. example if one just needs sqlite as a DB).

@driesvints
Copy link
Member

@jf-prevost thank you. Can you send in a PR?

@jf-prevost
Copy link
Contributor

@driesvints done :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants