Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove need to run invoke update after docker pull #6524

Open
wants to merge 26 commits into
base: master
Choose a base branch
from

Conversation

matmair
Copy link
Member

@matmair matmair commented Feb 19, 2024

This PR adds a new script contrib/container/execute.sh that checks if the API version of the starting image is different to the last checked version of the host and triggers invoke update automatically.

Fork details of 6305

Adoption of #6305, keeping the original commit by @ChristianSchindler to give co-authorship.

Removes the section we would not merge due to the comments brought up in #6305 (comment).
Keeps the great implementation of a database version check and auto-run of inv update - making docker setup much easier.

Ref #6303

@matmair matmair added docker Docker / docker-compose setup Relates to the InvenTree setup / installation process labels Feb 19, 2024
@matmair matmair self-assigned this Feb 19, 2024
Copy link

netlify bot commented Feb 19, 2024

Deploy Preview for inventree-web-pui-preview canceled.

Name Link
🔨 Latest commit c46fd85
🔍 Latest deploy log https://app.netlify.com/sites/inventree-web-pui-preview/deploys/6729a6e417e19c0008361a98

@matmair matmair changed the title Remvoe need to run invoke update after docker pull Remove need to run invoke update after docker pull Feb 19, 2024
@ChristianSchindler
Copy link

For the db_version we shud properly replace it with InvenTree/InvenTree/version.py and ther shut also be a possibility to deactivate auto update with an environment variable

@matmair matmair added this to the 0.15.0 milestone Feb 26, 2024
@matmair
Copy link
Member Author

matmair commented Feb 26, 2024

This will have to be pushed to 0.15.0, the release window for 0.14.0 is to close

@ChristianSchindler
Copy link

@matmair can we use InvenTree/InvenTree/version.py for getting the version?

@SchrodingersGat SchrodingersGat modified the milestones: 0.15.0, 0.16.0 Apr 25, 2024
@SchrodingersGat SchrodingersGat modified the milestones: 0.16.0, 0.17.0 Aug 5, 2024
@matmair
Copy link
Member Author

matmair commented Aug 20, 2024

This is working on my dev setup; would be happy for a review / general options @wolflu05 @SchrodingersGat

@matmair matmair marked this pull request as ready for review August 20, 2024 11:12
Copy link

@ChristianSchindler ChristianSchindler left a comment

Choose a reason for hiding this comment

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

Only one question for me is open. What do you want to do with the options to invoke an update:

Options:
  -f, --frontend      Force frontend compilation/download step (ignores INVENTREE_DOCKER)
  -k, --skip-static   Skip static file collection step
  -n, --no-frontend   Skip frontend compilation/download step
  -s, --skip-backup   Skip database backup step (advanced users)
  -u, --uv            Use UV (experimental package manager)

The good thing would be adding another env, or so where you can add them to every invoke update in the script.

But in general, thanks for completing my idea really grade worker

contrib/container/execute.sh Show resolved Hide resolved
contrib/container/init.sh Show resolved Hide resolved
contrib/container/init.sh Show resolved Hide resolved
Copy link
Contributor

@wolflu05 wolflu05 left a comment

Choose a reason for hiding this comment

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

LGTM, this will simplify setups drastically, as there is no command that needs to be run manually.

Regarding @ChristianSchindler's comment, I do not really see the need for using the options from the update command in the docker container.

  • the frontend commands should not be used as the frontend is already prebundeled in the container and any manual download or build will be ignored anyway
  • skipping static collection doesn't make any sense either
  • and using uv, think that doesn't work in the container either?
  • and skipping backup, mh, maybe, but I dont think that hurts

Having an option to disable the automatic update process may be worth it, but not sure.

@ChristianSchindler
Copy link

  • and skipping backup, mh, maybe, but I dont think that hurts

Personally I have to skip it because I use an unsupported version of postgres (postgres:16) and this is the only thing what is not working

@matmair
Copy link
Member Author

matmair commented Aug 23, 2024

I do not plan to support unsupported configs

@ChristianSchindler
Copy link

ChristianSchindler commented Aug 23, 2024

I do not plan to support unsupported configs

That is way I only find it good to have the same possibly in the container as without it. I think you had also different reasons way you cracked this options for this script I do not think about

@SchrodingersGat
Copy link
Member

SchrodingersGat commented Nov 5, 2024

@matmair in your opinion is this ready to merge?

  • Should we update the docker installation documentation to match?
  • Is there any unit testing we can apply here?

@matmair
Copy link
Member Author

matmair commented Nov 5, 2024

This needs better testing and should be rewritten to use the app version as a reference for current thing.

@matmair matmair modified the milestones: 0.17.0, 1.0.0 Nov 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docker Docker / docker-compose setup Relates to the InvenTree setup / installation process
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants