Conversation
The sdk was not listed as dependency of the server package although it is used in the server extensively. As we have for now no proper cross-package dependency defninition strategy, we apply the same workaround as with the compliance_tool. As the server is never released as package to PyPI but only install in docker image builds, this can be considered as appropiate temporary solution.
Previously the docker builds needed the repository root as build context in order to copy the content of sdk and server directory. In order to isolate the server package in its `/server` subpath, the build now uses this directory as context. The sdk sources are passed via an additional build context. CI jobs, docker-compose.yml files and Readmes are adapted accordingly.
…-dependency Also copy LICENSE to server directory in CI because this PR restricts the build context to the `server` directory. As consequence the build fails, when no LICENSE file is present in the server directory. For local building a copy instruction was added to the `server/README.md`.
|
I had to adapt the CI to copy the |
Previously, the example configuration READMEs still stated that the build context must be the repository root, `CONTRIBUTING.md` showed an outdated `docker build` command, and the Docker Hub link in the server `README.md` pointed to the old `basyx-python-server` image. This change updates them to match the build introduced in this branch: images are built from the `server` directory with the `sdk` directory passed in as a named build context. The registry README also pointed to the repository Dockerfile, which is corrected as well.
|
|
||
| COPY --from=sdk . /sdk | ||
| COPY ./app /server/app | ||
| COPY ./LICENSE /LICENSE |
There was a problem hiding this comment.
The LICENSE file only exists in ./LICENSE when the CI is ran, correct?
Wouldn't this mean this fails when trying to build the images on a local machine, for example with the compose.yaml files?
| ] | ||
| readme = "README.md" | ||
| license = "MIT" | ||
| license-files = ["LICENSE"] |
There was a problem hiding this comment.
Wouldn't this expect /server/LICENSE? The CI copy action puts the file at /LICENSE.
In the first version of these changes the `LICENSE` file from the repository root was copied in a CI step to the `/server` directory to include it into the Docker image builds. This lead to failing local builds on dev machines. To fix this the builds of all three server profiles now depend on a second additional build context `license`, which must hold the `LICENSE` file. Because Docker only allows directorie to be passed as additional build contexts, the `license` context is set to the repository root. These changes are documented in the contribution guide and all `README`s of the server's example configurations.
|
I changed include of the |
The server package uses the sdk extensively but never declared it as a dependency, and its Docker
builds required the repository root as build context, which is awkward for isolating the server
package in its own subpath.
basyx-python-sdkas an explicit dependency of the server package, using the samecommit-pinning workaround already applied to
compliance_tool. This is only a temporary fixuntil proper cross-package dependency definitions are figured out in Adopt a proper cross-package dependency strategy for the monorepo (evaluate
uvworkspaces) #592, which should beconsidered next.
./serveras build context instead of the repository root,passing the sdk sources in via a separate named build context.
Fixes #459
Fixes #470