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 new Docker concept #935

Merged
merged 4 commits into from
Mar 25, 2024
Merged

Conversation

piotrkardasz
Copy link
Contributor

@piotrkardasz piotrkardasz commented Sep 30, 2023

The new concept allows to setup environment with just one command 🚀

make init # initialize Sylius installation with required dependencies

ENV=test make run # to change the environment to test

make debug # to run php container with xDebug + mailhog

Requirements

  • Docker
  • Makefile

used dependencies

@probot-autolabeler probot-autolabeler bot added the Docker Docker-related issues and PRs. label Sep 30, 2023
.docker/ports.yaml Outdated Show resolved Hide resolved
compose.override.dist.yml Outdated Show resolved Hide resolved
@piotrkardasz piotrkardasz force-pushed the new-docker-concept branch 2 times, most recently from 231c23f to a81b46f Compare February 2, 2024 19:52
@Wojdylak Wojdylak marked this pull request as ready for review February 22, 2024 13:34
@Wojdylak Wojdylak requested a review from a team as a code owner February 22, 2024 13:34
Copy link
Contributor

@jakubtobiasz jakubtobiasz left a comment

Choose a reason for hiding this comment

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

IMO there's a lot of things happening in the compose files. Maybe we could introduce some "abstraction" by using custom Dockerfiles. For example, my compose files I'm using are +/- like this:

services:
    php:
        extends:
            file: /Users/jacob/Docker/compose/php.yaml
            service: frankenphp
        env_file:
            - .env.local
        environment:
            APP_URL: sylius.orb.local
        labels:
            - dev.orbstack.domains=sylius.orb.local
        volumes:
            - app:/app:delegated
        depends_on:
            - db
    db:
        extends:
            file: /Users/jacob/Docker/compose/database.yaml
            service: mariadb
        volumes:
            - database:/var/lib/mysql
        labels:
            - dev.orbstack.domains=db-sylius.orb.local
    frontend:
        extends:
            file: /Users/jacob/Docker/compose/common.yaml
            service: node
        volumes:
            - app:/app
    mailhog:
        image: mailhog/mailhog
        labels:
            - dev.orbstack.domains=mailhog-sylius.orb.local

Some things are due to the fact I'm using the Orbstack, but we should definitely consider making the Docker experience as simple as we can.

From our experience, not every person touching Sylius with Docker is familiar with it.

compose.yml Outdated Show resolved Hide resolved
compose.override.dist.yml Outdated Show resolved Hide resolved
compose.override.dist.yml Outdated Show resolved Hide resolved
compose.debug.yml Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
version: "3.8"
services:
php:
image: ghcr.io/sylius/sylius-php:8.2-fixuid-xdebug-alpine
Copy link
Contributor

Choose a reason for hiding this comment

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

Using an exterior image requires some customisation documentation

@vasilvestre
Copy link
Contributor

It's my own opinion but I'm not fan of the external tool dependency (fixuid) which isn't even 1.0 released.

@piotrkardasz
Copy link
Contributor Author

It's my own opinion but I'm not fan of the external tool dependency (fixuid) which isn't even 1.0 released.

fixuid is reasonable way for DX development to handle correct UIDs and GIDs for Linux/Windows(WSL); for Mac it is redundant.
⚠️ Fixuid should be used only for development purposes

@jakubtobiasz jakubtobiasz merged commit ceb354c into Sylius:1.12 Mar 25, 2024
8 checks passed
@piotrkardasz piotrkardasz deleted the new-docker-concept branch March 25, 2024 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Docker Docker-related issues and PRs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants