search

SEMICeu / iso-19139-to-dcat-ap

Reference XSLT-based implementation of GeoDCAT-AP
European Union Public License 1.2
15 stars 9 forks source link

Dockerfied #3

Closed leu2tm closed 3 years ago

leu2tm commented 3 years ago
andrea-perego commented 3 years ago

@leu2tm , thanks for your contribution.

Could you please provide some context for this PR?

leu2tm commented 3 years ago

@andrea-perego Of course!

This PR is made for an easy way to start this "app" without the knowledge of PHP requirements, plugins etc.

Also since then there have been some changes to the 3rd party dependency easyrdf, so refactoring has been made to make it function again.

Also the version of the dependency has been fixed in the composer.json instead of latest/master, so there won't be any future breaking changes.

And at last, the docker version uses one of the latest versions of PHP to run the "app".

/ leu2tm

andrea-perego commented 3 years ago

Thanks, @leu2tm . It would also be interesting to know if this Docker installation has been deployed somewhere, and for which use case.

About upgrading to more recent versions of EasyRDF, this is also being addressed in a separate branch (along with other things):

https://github.com/SEMICeu/iso-19139-to-dcat-ap/tree/upgrade-api-script

Coming to your PR, I see some issues with the following changes:

  1. api/Dockerfile:

    + RUN sed -ri -e 's!/var/www/html!${APACHE_DOCUMENT_ROOT}!g' /etc/apache2/sites-available/*.conf
+ RUN sed -ri -e 's!/var/www/!${APACHE_DOCUMENT_ROOT}!g' /etc/apache2/apache2.conf /etc/apache2/conf-available/*.conf

    Are these two sed commands making changes to the Apache config files of the hosting machine? If this is the case, it would be safer to install Apache inside the Docker container, and change the configuration there. Otherwise, access to other possibly hosted services, available from the original Apache DOCUMENT_ROOT, may be broken.

  2. api/index.php:

    - require('./lib/composer/vendor/autoload.php');
+ require('/vendor/autoload.php');

    I have not tested it, but the change of path may not work with the current PHP basic installation. It would be better to edit the copy of the index.php file in the Docker container, and not the original one.

leu2tm commented 3 years ago

@andrea-perego I would say this is just a guided way for setting it up for a local development environment.

  1. No, the docker commands will only affect the container when spun up from the image. So no files from the host machine will be touched, except copied into the image.

Also the image is currently running on a php image with apache installed, so no worries.

FROM php:7.4-apache

https://github.com/SEMICeu/iso-19139-to-dcat-ap/pull/3/files#diff-21ee93e31c9cec7a5f33b680622da377c451b62b6c44eac6d9550eade41beb47R8

  1. I will update it, so it will work with the previous setup.

EDIT:

It should be working now without interferring with the previous setup.

andrea-perego commented 3 years ago

Thanks, @leu2tm .

I'm going to merge this PR.

Thanks again for this contribution.