bitflicker64 commented on PR #3025: URL: https://github.com/apache/hugegraph/pull/3025#issuecomment-4470141101
> Thanks for iterating on the Docker cache changes. The cache direction looks good, and besides the monitor/runtime compatibility point below plus the small `.dockerignore` typo, I don't see other blockers from my side. > Thanks for the review !! > I think we should keep the runtime behavior unchanged in this PR. > > Today the image still exposes the legacy monitor path: > ........... > ........... > ........... > But this is a runtime behavior/design change, not just a build-cache optimization. It also needs documentation, because standalone Docker healthcheck only marks the container as unhealthy and does not restart it by itself; users still need a restart policy, or an entrypoint watchdog that exits the container after repeated failures. > Makess. sensee > So for this PR, I suggest the lowest-risk path: > > 1. Keep `cron` in the runtime image for now, so the existing `start-hugegraph.sh -m true` monitor path is not broken. > 2. Remove only the build-time `service cron start` cleanup if needed, since starting a service during image build does not keep it running in containers. > 3. Fix the small `.dockerignore` typo: `.gitattribut` -> `.gitattributes`. > 4. Leave the Docker-native watchdog / healthcheck / restart-policy design to a follow-up PR with docs. > should i also remove `**/target/dist/` since `**/target/` is alr covered also wanted to ask about `**/*.tar.gz*` should we keep it `**/*.tar.gz` to be safee ? > In short: > > ``` > This PR: > improve Docker build cache > keep runtime behavior unchanged > > Follow-up PR: > replace legacy cron monitor with Docker-native watchdog / healthcheck docs > ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
