Skip to content

Conversation

@devinmatte
Copy link
Member

We're getting ready to move far away from Yasuko for almost all web projects. This is the first stage in getting schedulemaker off of Yasuko, and onto openshift.

We're first starting by moving schedulemaker-dev. Currently everything except for running the cron for parsing. That's in the works and should be done soon.

@devinmatte devinmatte self-assigned this Apr 16, 2018
Copy link

@stevenmirabito stevenmirabito left a comment

Choose a reason for hiding this comment

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

A bit of Dockerfile refactoring but otherwise looks good. In general, try to minimize the number of statements in your Dockerfile to minimize the number of layers, which helps with image caching.

$DATABASE_USER = $get_env(getenv("DATABASE_USER"), '');
$DATABASE_PASS = $get_env(getenv("DATABASE_PASS"), '');
$DATABASE_DB = $get_env(getenv("DATABASE_DB"), '');
$DUMPCLASSES = '/mnt/share/cshclass.dat';

Choose a reason for hiding this comment

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

These will also likely need to be configurable, with a default such as /data

Dockerfile Outdated
FROM php:7.1-apache
MAINTAINER Devin Matte <matted@csh.rit.edu>

EXPOSE 8080

Choose a reason for hiding this comment

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

EXPOSE statements are typically at the end of the file

Dockerfile Outdated
ADD apache-config.conf /etc/apache2/sites-enabled/000-default.conf

RUN apt-get -yq update && \
apt-get -yq install gnupg libmagickwand-dev --no-install-recommends

Choose a reason for hiding this comment

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

Need to run apt-get -yq clean all here

Dockerfile Outdated

RUN a2enmod rewrite && a2enmod headers && a2enmod expires

RUN sed -i '/Listen/{s/\([0-9]\+\)/8080/; :a;n; ba}' /etc/apache2/ports.conf

Choose a reason for hiding this comment

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

Combine both of these RUN statements, along with the RUN statement at the end of the file (do all Apache config in a single RUN statement)

Dockerfile Outdated
apt-get -yq install gnupg libmagickwand-dev --no-install-recommends

RUN docker-php-ext-install mysqli
RUN pecl install imagick && docker-php-ext-enable imagick

Choose a reason for hiding this comment

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

Combine these RUN statements

Dockerfile Outdated
npm install && \
npm run-script build && \
rm -rf node_modules && \
apt-get -yq remove nodejs npm && \

Choose a reason for hiding this comment

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

You're not installing npm, does the package stay behind when nodejs is removed? Otherwise, remove it here.

Copy link

@stevenmirabito stevenmirabito left a comment

Choose a reason for hiding this comment

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

🚢 🎉

@devinmatte devinmatte merged commit 1451c63 into ComputerScienceHouse:develop Apr 17, 2018
@devinmatte devinmatte deleted the openshift branch October 11, 2019 15:22
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.

2 participants