Skip to content

Conversation

Sparrow1029
Copy link

This PR adds a SQL dump of the base netbox schema to docker/postgresql/init.sql to decrease startup time. It also adds the ability to overload the image used by orchestrator and orchestrator-frontend services via ORCH_BACKEND_TAG and ORCH_UI_TAG compose variables.

requires-python = ">=3.12"
dependencies = [
"deepdiff==8.0.1",
"orchestrator-core==4.0.4",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was discussing this with Thomas yesterday, do you think there's any reason to keep the core version specified here? It was in the requirements.txt before which historically was added to specify extra dependencies, but orchestrator-core is already in the docker image so I'm not sure why we'd want to install it again. I'd say we just remove this line?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It depends. This dependency section was generated via uv add -r requirements.txt from the requirements.txt file in the project root. I think this relates to your other comment, in that if we switch to installing via uv in the entrypoint.sh script, we will need to

  1. Make sure the orchestrator-core Docker images on ghcr.io include uv executable
  2. Change the RUN commands for the build in the Dockerfile to be compatible with uv's process, if that's the direction we want to go

uv docs for Docker
uv-docker-example project Dockerfile

@@ -1,3 +1,14 @@
[project]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would make sense to me if the entrypoint.sh script installed dependencies with uv instead of requirements.txt, what do you think?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my other comment for reference

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants