-
Notifications
You must be signed in to change notification settings - Fork 5
Conversation
Pipfile
Outdated
@@ -9,6 +9,7 @@ pytest = "~=3.7.1" | |||
[packages] | |||
flask = "~=1.0.2" | |||
flask-sqlalchemy = "*" | |||
uwsgi = "*" |
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.
These unbounded constraints are a bad idea.
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.
It is only the default, we should pin on at least 2.*
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.
Who's default, Pipfile?
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.
Pipenv.lock
fixes the version. There are examples in the Pipenv documentation and the Pipfile project.
I think the recommendation is to only specify the versions we know we require (e.g. known minimum version). You can then still install dependencies as per the lock file for deterministic builds and use the Pipfile to upgrade dependencies. There may be some official documentation explaining that more.
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.
Default of pipenv install $packagename
; I have now specified 2.0.*
which avoids surprises when upgrading dependencies
Pipfile.lock
Outdated
@@ -100,15 +107,15 @@ | |||
"sha256:6e3836e39f4d36ae72840833db137f7b7d35105079aee6ec4a62d9f80d594dd1", | |||
"sha256:95eb8364a4708392bae89035f45341871286a333f749c3141c20573d2b3876e1" | |||
], | |||
"markers": "python_version != '3.1.*' and python_version >= '2.7' and python_version != '3.3.*' and python_version != '3.2.*' and python_version != '3.0.*'", | |||
"markers": "python_version >= '2.7' and python_version != '3.1.*' and python_version != '3.2.*' and python_version != '3.3.*' and python_version != '3.0.*'", |
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.
Pipfile producing different results again?
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'm using https://github.com/libero/content-store/pull/13/files#diff-04c6e90faac2675aa89e2176d2eec7d8R35 so that it becomes consistent
README.md
Outdated
docker-compose run --name my_install venv pipenv install uwsgi | ||
docker cp my_install:/Pipfile.lock Pipfile.lock | ||
docker cp my_install:/Pipfile.lock Pipfile.lock | ||
docker rm my_install |
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.
Should use a Docker Compose override
instead.
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.
pipenv
cannot write to a mounted Pipfile.lock
because it creates a new file in place of the old one. Could probably try mounting the whole folder 🤔
@@ -0,0 +1,3 @@ | |||
from content_store.api.api import create_app |
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.
The app shouldn't be in the api
namespace, let alone twice!
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.
General structure (until there is logic to organize) still unclear, but a few (hopefully one) stable top-level package(s) is needed, the rest can be moved around
Run into pypa/pipenv#2760 but not harmful. |
Dockerfile.venv
Outdated
@@ -1,9 +1,14 @@ | |||
ARG python_base_image_tag | |||
FROM python:${python_base_image_tag} | |||
|
|||
# uwsgi build process dependencies | |||
RUN apk update && apk add gcc libc-dev linux-headers |
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.
--no-cache
Dockerfile.venv
Outdated
@@ -1,9 +1,14 @@ | |||
ARG python_base_image_tag | |||
FROM python:${python_base_image_tag} | |||
|
|||
# uwsgi build process dependencies | |||
RUN apk update && apk add gcc libc-dev linux-headers | |||
RUN pip install --no-cache-dir --only-binary --upgrade pipenv |
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.
Should merge the two RUN
s.
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.
Can do, although there's not particular trigger that would re-execute these outside of the package names
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.
(can even merge 3)
docker-compose.yml
Outdated
environment: | ||
- FLASK_APP=content_store/api/api.py | ||
command: flask run --host=0.0.0.0 | ||
# TODO: check what to use for uwsgi |
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.
Was this resolved?
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.
It works better with the current configuration, as the default SIGTERM
is not handled. This can usually be tweaked with the uwsgi configuration but, not being in the container image, I want to see a real one first.
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.
(by default it will handle SIGINT
and SIGQUIT
)
All changes incorporated. |
Another Pipfile.lock conflict... |
Configuration of both uWSGI (for local usage) and of the Docker container needs cleaning, but I can see
/ping
working.Last part of libero/publisher#31