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

Chore/sc 117667/upgrade node version for dp produc #61

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

JakeBiddell
Copy link

No description provided.

@JakeBiddell JakeBiddell changed the title Chore/sc 117667/upgrade node version for dp product jake Chore/sc 117667/upgrade node version for dp produc Aug 28, 2024
@@ -5,4 +5,4 @@ docker-product-base:
FROM DOCKERFILE -f ./Dockerfile ./
ARG EARTHLY_TARGET_TAG_DOCKER
ARG IMAGE_TAG=$EARTHLY_TARGET_TAG_DOCKER
SAVE IMAGE --cache-from deskpro/docker-product-base:main deskpro/docker-product-base:$IMAGE_TAG
SAVE IMAGE --cache-from deskpro/docker-product-base:chore/sc-117667/upgrade-node-version-for-dp-product-jake deskpro/docker-product-base:$IMAGE_TAG
Copy link
Member

Choose a reason for hiding this comment

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

Don't do this; after merge this won't be correct. Keep it as :main

@@ -56,6 +56,10 @@ else
fi

main() {
# store the fact that we're running the entrypoint
Copy link
Member

Choose a reason for hiding this comment

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

There's already a sentinel file for a similar purpose at /run/container-ready that is-ready already checks. Does it not work as intended?

I would not think it's super useful to know if entrypoint specifically is running or not. It's more useful to know that services are actually running, which is the purpose of that sentinel file.

@JakeBiddell JakeBiddell marked this pull request as draft August 28, 2024 15:38
@JakeBiddell JakeBiddell force-pushed the chore/sc-117667/upgrade-node-version-for-dp-product-jake branch from 271402f to cf4a15e Compare September 10, 2024 22:02
@JakeBiddell JakeBiddell force-pushed the chore/sc-117667/upgrade-node-version-for-dp-product-jake branch from cf4a15e to 4942e38 Compare October 24, 2024 08:47
@JakeBiddell JakeBiddell force-pushed the chore/sc-117667/upgrade-node-version-for-dp-product-jake branch from 4942e38 to b0d20ab Compare November 1, 2024 11:33
Add is-ready check on the default files tests
Add a check the entrypoint is not still running (slightly different case to the ready file that gets added on first boot)
@JakeBiddell JakeBiddell force-pushed the chore/sc-117667/upgrade-node-version-for-dp-product-jake branch from b0d20ab to beb7a65 Compare November 12, 2024 12:23
@JakeBiddell JakeBiddell force-pushed the chore/sc-117667/upgrade-node-version-for-dp-product-jake branch from beb7a65 to 5c3a065 Compare November 12, 2024 12:29
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