-
Notifications
You must be signed in to change notification settings - Fork 42
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
Better set up for docker-compose #636
Conversation
- Remove annif/nginx directory - Write nginx.conf with Docker run command - Remove unnecessary bash container - Better name for Annif container
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
gunicorn_server: | ||
image: quay.io/natlibfi/annif | ||
annif_app: | ||
image: quay.io/natlibfi/annif:latest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see that this changed to :latest
, was this intended?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the tag is latest
by default; this just makes the behaviour more explicit.
Codecov ReportBase: 99.58% // Head: 99.58% // No change to project coverage 👍
Additional details and impacted files@@ Coverage Diff @@
## master #636 +/- ##
=======================================
Coverage 99.58% 99.58%
=======================================
Files 87 87
Lines 5990 5990
=======================================
Hits 5965 5965
Misses 25 25 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, although I didn't test it
On second thought, would it be better to remove also the |
My slight preference would be to keep it as a file in the main repo. I see your point, but I think it's better to keep the file in a visible place under version control, and also being a part of Annif releases. |
I updated the Wiki page to align to these changes. |
The NGINX proxy server link on that page seems outdated, probably it should be switched to https://nginx.org/en/docs/ or some other suitable resource |
Link switched to the up-to-date one. |
The docker-compose setup has not been really used much. It is mentioned in the Docker Wiki page mostly as a step towards production environment. To make NGINX proxy requests to Annif, the NGINX configuration has been written in the
annif/nginx/nginx.conf
file, which quite uselessly clutters the annif directory.However there is an alternate solution: creating the config file with Docker cmd from content written inline in the
docker-compose.yaml
.Changes:
annif/nginx
directory with the config filenginx.conf
with Docker run command