Skip to content

Conversation

@Bill-Becker
Copy link
Collaborator

@Bill-Becker Bill-Becker commented Sep 2, 2025

@Bill-Becker Bill-Becker marked this pull request as ready for review September 2, 2025 17:09
@Bill-Becker Bill-Becker requested a review from Copilot September 2, 2025 17:09
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the Kubernetes deployment configuration to combine Celery and Julia containers into a single pod and introduces a cron job for automated pod restarts. The changes optimize resource allocation and improve deployment management.

Key changes:

  • Combines Celery and Julia services into a single deployment pod
  • Reduces resource allocation across multiple configuration files
  • Adds automated Jenkins pipeline for periodic pod restarts

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
julia_src/http.jl Adds access logging to Julia HTTP server
config/gunicorn.conf.py Reduces worker count from 8 to 4 for K8S deployments
Jenkinsfile-restart-celery-julia New Jenkins pipeline for automated pod restarts via cron
.helm/values.yaml Updates resource requests and reduces Julia replicas
.helm/values.staging.yaml Removes redundant Julia CPU configuration
.helm/values.production.yaml Reduces Julia replicas and memory allocation
.helm/templates/celery-deployment.yaml Adds Julia container to Celery deployment pod

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +57 to +59
env:
- name: JULIA_HOST
value: "localhost"
Copy link

Copilot AI Sep 2, 2025

Choose a reason for hiding this comment

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

Setting JULIA_HOST to 'localhost' hardcodes the assumption that Julia runs in the same pod. Consider using a configurable value or documenting this coupling to avoid confusion during future maintenance.

Suggested change
env:
- name: JULIA_HOST
value: "localhost"
value: {{ quote .Values.juliaHost }}
# NOTE: The default value assumes Julia runs in the same pod as Celery.
# Change .Values.juliaHost in values.yaml if Julia is deployed elsewhere.

Copilot uses AI. Check for mistakes.
HTTP.register!(ROUTER, "GET", "/get_ashp_defaults", get_ashp_defaults)
HTTP.register!(ROUTER, "GET", "/pv_cost_defaults", pv_cost_defaults)
HTTP.serve(ROUTER, "0.0.0.0", 8081, reuseaddr=true)
HTTP.serve(ROUTER, "0.0.0.0", 8081, reuseaddr=true, access_log=combined_logfmt)
Copy link

Copilot AI Sep 2, 2025

Choose a reason for hiding this comment

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

The variable 'combined_logfmt' is referenced but not defined in this diff. Ensure this variable is properly imported or defined elsewhere in the file to avoid runtime errors.

Copilot uses AI. Check for mistakes.
@Bill-Becker Bill-Becker merged commit 006f7c4 into master Sep 2, 2025
3 checks passed
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