Conversation
| jinja2 = ">=2.8" | ||
| click = ">=6.6" | ||
| terminaltables = ">=3.0.0" |
There was a problem hiding this comment.
Given these are only for the cli extras_require in the setup.py, these should be similarly specified in Poetry using an optional group: https://python-poetry.org/docs/managing-dependencies/#optional-groups so that folks installing without need for the CLI functionality can ignore them and install only the library.
| readme = "README.md" | ||
|
|
||
| [tool.poetry.dependencies] | ||
| python = "^3.9" |
There was a problem hiding this comment.
This specification resolves to >=3.9.0, <4.0.0. I'd imagine that can be loosened up a bit; the effective minimum version of python for this library definitely isn't 3.9.
| expose: | ||
| - "8011" |
There was a problem hiding this comment.
expose exposes this port to other services in this Docker network (eg, defined in this docker-compose.yml file). Since you have none, and so no other containers that will want to talk to this one, this isn't necessary.
| @@ -0,0 +1,5 @@ | |||
| FROM capless/capless-docker:jupyter | |||
| COPY . /code | |||
| RUN python -m pip install --upgrade poetry | |||
There was a problem hiding this comment.
FYI this is not Poetry's recommended method of installing Poetry: https://python-poetry.org/docs/#installation
| FROM capless/capless-docker:jupyter | ||
| COPY . /code | ||
| RUN python -m pip install --upgrade poetry | ||
| RUN poetry run pip install --upgrade pip |
There was a problem hiding this comment.
Modern versions of Poetry use an internal installer and not pip anymore, so this may not be necessary.
| @@ -0,0 +1,19 @@ | |||
| [tool.poetry] | |||
| name = "envs" | |||
| version = "0.1.0" | |||
There was a problem hiding this comment.
This should follow the version of the library in the setup.py, and be updated for new releases. setup.py is currently at 1.3, though current version in PyPI is 1.4. This is why using VCS tags and associated tooling is helpful a la #21
| @@ -0,0 +1,5 @@ | |||
| FROM capless/capless-docker:jupyter | |||
| COPY . /code | |||
There was a problem hiding this comment.
Having this COPY before the Poetry install will cause Docker's layer caching to assume it needs to reinstall poetry whenever the source code changes. Moving this after installing Poetry will allow that layer to be cached and save time on subsequent builds.
| volumes: | ||
| - ./sammy/:/code/sammy |
There was a problem hiding this comment.
This may have been copy-pasta'ed from whatever example you used, but wouldn't work based on directories in this repo. Looking at the Dockerfile I"m imagining what you want is
| volumes: | |
| - ./sammy/:/code/sammy | |
| volumes: | |
| - .:/code/ |
Changes
master