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 drupal-with-postgresql service template #2463

Merged
merged 3 commits into from
Jul 15, 2024

Conversation

emircanerkul
Copy link
Contributor

No description provided.

@JPustkuchen
Copy link

Thanks, really nice idea to have a Drupal template, but please let me point out that the current directory structure used does not use the Drupal standards:

See https://www.drupal.org/download > https://packagist.org/packages/drupal/recommended-project > https://github.com/drupal/recommended-project/blob/6b99c737444f91c14e4b5e9034fd6844415989ba/composer.json#L48

        "drupal-scaffold": {
            "locations": {
                "web-root": "web/"
            }
        },
        "installer-paths": {
            "web/core": ["type:drupal-core"],
            "web/libraries/{$name}": ["type:drupal-library"],
            "web/modules/contrib/{$name}": ["type:drupal-module"],
            "web/profiles/contrib/{$name}": ["type:drupal-profile"],
            "web/themes/contrib/{$name}": ["type:drupal-theme"],
            "drush/Commands/contrib/{$name}": ["type:drupal-drush"],
            "web/modules/custom/{$name}": ["type:drupal-custom-module"],
            "web/profiles/custom/{$name}": ["type:drupal-custom-profile"],
            "web/themes/custom/{$name}": ["type:drupal-custom-theme"]
        },

We should please stick to Drupal standards, when preparing such a service template.

@sandros94
Copy link

sandros94 commented Jul 3, 2024

@JPustkuchen Could I ask you to elaborate on that?
Are you referring that the main folder is not /var/www/html/** but rather /opt/drupal/web/**?

EDIT:
just double checked, the official Drupal docker page does state to use /var/www/html/** for persistent storage

@JPustkuchen
Copy link

@sandros94 I'm not using coolify yet. I can take a look at it, once I have the time. But to answer the question and unblock the issue here perhaps, I can tell the following:

Drupal creates a certain folder structure when following the proposed composer installation (see link) and the coolify structure should follow that one.

Update: Just saw your EDIT link. That's strange and I think it seems to be kind of inconsistency I can't understand (on the Drupal side). Some years ago Drupal used that /html/ structure, but that's long ago.

So I posted a comment at docker-library/drupal#3 (comment)

So the implementation here seems correct so far, based on the docker documentation, but maybe it's outdated upstream.

@sandros94
Copy link

That's strange and I think it seems to be kind of inconsistency I can't understand (on the Drupal side)

Yeah, it is an inconsistency. Like for example if you want to seed a Drupal site you must first do it with another container or full build a custom image. Easily doable, but still not as intuitive as other projects.

So I posted a comment at docker-library/drupal#3 (comment)

Just saw you comment popping up as I was reading that issue. To me it does seem like an upstream refactoring is required.

@emircanerkul
Copy link
Contributor Author

Hi @JPustkuchen I just followed drupal docker instructions while creating that template.

Please see in volumes section https://hub.docker.com/_/drupal

@JPustkuchen
Copy link

Okay, my assumption and confusion seems to have been wrong, as this comment points out: docker-library/drupal#3 (comment)

I didn't see the link mapping this to the /html folder! Sorry for the confusion, so I think this is looking good, but as written above I didn't try it myself yet, so further feedback is welcome.

@andrasbacsai
Copy link
Member

Thank you for the PR!

@andrasbacsai andrasbacsai merged commit eb3a4ca into coollabsio:next Jul 15, 2024
1 check passed
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