Skip to content

Update readme file & Docker Setup#12

Merged
roschaefer merged 8 commits intomasterfrom
11-update-readme-file
Jan 24, 2019
Merged

Update readme file & Docker Setup#12
roschaefer merged 8 commits intomasterfrom
11-update-readme-file

Conversation

@matboehm
Copy link
Copy Markdown
Contributor

@matboehm matboehm commented Jan 23, 2019

Updated for the README.md documentation and a first basic version of a dockerized API server.

Currently Work in Progress!

@roschaefer
Copy link
Copy Markdown
Contributor

@matthib you can prefix the PR title with [WIP] to indicate a work-in-progress pull request.

Copy link
Copy Markdown
Contributor

@roschaefer roschaefer left a comment

Choose a reason for hiding this comment

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

I like it, you're going in the right direction @matthib - please add a separate #Installation with Docker section and keep the verbosity low. E.g. you can remove redundancy by showing the terminal output instead of a text paragraph explaining the command which the user should put in the terminal.

Comment thread README.md

**PREREQUESITES**

Before starting the installation you need to make sure you have the following tools installed:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Slightly verbose. I would put this paragraph in one sentence and use link formatting. Example:

"""
Make sure that you have a recent version of node, yarn, EmberJS, ruby, Redis and postgresql installed before you proceed. E.g. we have the following versions:

$ node --version
v10.4.1
$ yarn --version
1.7.0
$ ember --version
ember-cli: 3.1.4
node: 10.4.1
os: linux x64

"""
source: https://github.com/roschaefer/rundfunk-mitbestimmen/edit/master/README.md

Comment thread README.md Outdated
Comment thread Dockerfile
@@ -0,0 +1,16 @@
FROM node:11
Copy link
Copy Markdown
Contributor

@roschaefer roschaefer Jan 23, 2019

Choose a reason for hiding this comment

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

Suggested change
FROM node:11
FROM node:10-alpine

Because of

  • Long term support
  • We can share the docker base image with Nitro-Backend and Nitro-Web

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately the alpine version of node doesn't work with the dependencies we have. We use the package "sharp", which needs several build tools that node-alpine doesn't include. Manually installing those requirements in the Dockerfile for node-alpine unfortunately didn't work out, so I was forced to use the full node image.
Should we downgrade to version 10 of node though?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ewwww

OK, let's just keep it for now

@matboehm matboehm changed the title Update readme file & Docker Setup [WIP] Update readme file & Docker Setup Jan 23, 2019
…g port number

Co-Authored-By: matthib <matthi@bonn-boehms.de>
@roschaefer
Copy link
Copy Markdown
Contributor

@matthib actually we can merge this PR, why is it work-in-progress? I just docker-compose up and the server is listening on http://localhost:1337/, getting:

// 20190123230807
// http://localhost:1337/

{
  "error": "unauthorised"
}

@roschaefer
Copy link
Copy Markdown
Contributor

For testing mail delivery during development, I suggest to use mailhog and configure SMTP settings in docker-compose.yml.

@appinteractive
Copy link
Copy Markdown
Member

Here could be a solution for using sharp with alpine.
libvips/libvips#1142

@matboehm
Copy link
Copy Markdown
Contributor Author

@matthib actually we can merge this PR, why is it work-in-progress? I just docker-compose up and the server is listening on http://localhost:1337/, getting:

// 20190123230807
// http://localhost:1337/

{
  "error": "unauthorised"
}

@roschaefer The database part is still missing. But you are right, the system works like this right out of the box.
For the database part I can make another ticket. Then I'll build a function that creates the database tables together with some test entries, so the API server can be used with the database and some seed data. Then I will just run the db creation and seeding process in the Docker build process.

@matboehm
Copy link
Copy Markdown
Contributor Author

matboehm commented Jan 24, 2019

Here could be a solution for using sharp with alpine.
libvips/libvips#1142

@appinteractive Thanks for the link! Tried those solutions before and unfortunately they didn't work out. I still get errors when trying to build sharp in the npm install process. I'll keep looking though, maybe there is a solution that works fine.

@roschaefer roschaefer merged commit 0202c71 into master Jan 24, 2019
@roschaefer roschaefer deleted the 11-update-readme-file branch January 24, 2019 21:53
@roschaefer
Copy link
Copy Markdown
Contributor

@matthib well then let's create another ticket

@matboehm matboehm changed the title [WIP] Update readme file & Docker Setup Update readme file & Docker Setup Jan 24, 2019
@pr-triage pr-triage Bot added the PR: merged label Jan 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants