RDFLib / prez

Prez is a data-configurable Linked Data API framework that delivers profiles of Knowledge Graph data according to the Content Negotiation by Profile standard.
BSD 3-Clause "New" or "Revised" License
18 stars 7 forks source link

Docker build and wheel dist build fixes #223

Closed ashleysommer closed 2 months ago

ashleysommer commented 2 months ago

I found some inconsistencies and bugs while building Prez in a docker container for deployment on Azure cloud.

This is a collection of five small commits related to building the Prez bdist wheel, installing it in the Docker container, and building the Docker container.

1 Use arg/env variable for venv creation instead of hardcoded location. (It used the venv in other calls, just not in the venv creation command). Explicitly call the venv version of pip3 to install the wheel, so it is obvious the built prez module is being installed in the venv site_packages.

2 Don't copy the whole source tree into the final image, it already has the built prez module and all dependencies installed in the venv, it just needs main.py and supporting files.

3 Never include dist files in the docker env, this also ensures no dist files are included inside the sdist or wheel packages (poetry excludes packaging everything in gitignore).

4 Don't put Dockerfile inside its own Docker build context. Its not needed for the container built, and this allows incremental builds to work properly.

5 Put prez module's pyproject.toml inside the built prez module in the wheel package, this allows the config builder to find it when prez is installed as a system module. Looks like this is what #216 was trying to solve. Modify the config builder to search inside the prez module for pyproject.toml if there is not one in the project top directory (enables the above functionality). Also explicitly list all the files to include in a sdist, because should be different than the wheel bdist.

lalewis1 commented 2 months ago

@ashleysommer and @edmondchuc. Hey guys, I just pulled these changes and when I try to run poetry install Poetry is complaining that:

The Poetry configuration is invalid:

Just checking if this is something that is happening to you both as well or if it is just me?

Python version == 3.11 Poetry version == 1.7.1

Note: I checked the poetry docs and I can't see why it would be unhappy with the 'to' declaration in pyproject.toml, Only thing I can think of is that maybe it has to have a 'from' declaration as well.

ashleysommer commented 2 months ago

Hi @lalewis1 I'm on Poetry version v1.8.1, and it works as expected in my testing.

I just checked the documentation, and I didn't realize that is a feature that was recently introduced in Poetry v1.8.0 https://python-poetry.org/docs/

More specifically, I found its a feature of poetry-core v1.9.0+ https://github.com/python-poetry/poetry-core/pull/672

It's not in the docs for Poetry v1.7, so it must've been added after that. I thought that was a basic packaging feature that has been around for years.

Do you guys have a policy on supported Poetry version? When building Prez in a docker container its not an issue because latest Poetry is always used inside the container. But is it reasonable to expect developers on the code to always have the latest Poetry version, or should we maintain some kind of backwards compatibility?

lalewis1 commented 2 months ago

I think it would be fine to have the dependency on the latest version. It would seem there is no way to add the requirement in pyproject.toml (https://github.com/python-poetry/poetry/issues/6088), so perhaps the best thing will be to add a note to the readme.

lalewis1 commented 2 months ago

I just upgrade poetry to 1.8.2 and poetry-core 1.9.0 and it is now working as expected for me as well.

Thanks for the quick investigation!

ashleysommer commented 2 months ago

Actually, you can specify the required poetry-core version used for the build backend here: https://github.com/RDFLib/prez/blob/0437c75a4c7e3620d38721f8d48f1087c86e073f/pyproject.toml#L47 Perhaps its worth bumping that to v1.9 now, then using that as the baseline minimum-supported poetry-core.

lalewis1 commented 2 months ago

yes good idea. I'll do it now.