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

enhancement(prefect-server): expose PREFECT_SERVER_API_BASE_PATH setting on server deployment #460

Merged
merged 5 commits into from
Mar 3, 2025

Conversation

jamiezieziula
Copy link
Contributor

@jamiezieziula jamiezieziula commented Feb 27, 2025

Summary

This PR exposes the ability to set the environment variable PREFECT_SERVER_API_BASE_PATH on the server deployment. This allows users to set a custom api path to use. This is not breaking, as the exposed value is set to the previous default of /api.

Additionally, when configuring this new setting, I realized that the background services deployment does not currently support readiness/liveness probe checks. I've removed them from the chart for now.

Requirements

  • Contributing guide has been read
  • Title follows the conventional commits format
  • Body includes Closes <issue>, if available
  • Added/modified configuration includes a descriptive comment for documentation generation
  • Unit tests are added/updated
  • Deprecation/removal checks are added in templates/NOTES.txt
  • Relevant labels are added
  • Draft status is used until ready for review

Relates to PrefectHQ/prefect#16967
Resolves https://linear.app/prefect/issue/PLA-1119/add-support-for-prefect-server-api-base-path-in-the-prefect-helm

@jamiezieziula jamiezieziula added the enhancement An improvement of an existing feature label Feb 27, 2025
@@ -107,20 +107,6 @@ spec:
{{- with .Values.backgroundServices.containerSecurityContext }}
securityContext: {{- toYaml . | nindent 12 }}
{{- end }}
{{- if .Values.backgroundServices.livenessProbe.enabled }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not currently supported. This is what you see when trying to configure this on the background services deployment today:

Events:
  Type     Reason     Age               From               Message
  ----     ------     ----              ----               -------
  Normal   Scheduled  34s               default-scheduler  Successfully assigned prefect/prefect-server-background-services-7bc4767c5d-4m29c to gke-node-node-node-node-0000-000000000-0000
  Normal   Pulled     33s               kubelet            Container image "prefecthq/prefect:3.2.7-python3.11" already present on machine
  Normal   Created    33s               kubelet            Created container prefect-background-services
  Normal   Started    33s               kubelet            Started container prefect-background-services
  Warning  Unhealthy  4s (x2 over 14s)  kubelet            Readiness probe failed: Get "http://10.0.0.10:4200/api/ready": dial tcp 10.0.0.10:4200: connect: connection refused
  Warning  Unhealthy  4s (x2 over 14s)  kubelet            Liveness probe failed: Get "http://10.0.0.10:4200/api/health": dial tcp 10.0.0.10:4200: connect: connection refused

@@ -150,7 +156,8 @@
"loggingLevel": {
"type": "string",
"title": "Logging Level",
"description": "sets PREFECT_LOGGING_SERVER_LEVEL"
"description": "sets PREFECT_LOGGING_SERVER_LEVEL",
"enum": ["DEBUG", "INFO", "WARNING", "ERROR", "CRITICAL"]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

based on the docs, i've added this as a check to ensure we're passing only correct values/

@@ -53,10 +53,16 @@ server:
# -- priority class name to use for the server pods; if the priority class is empty or doesn't exist, the server pods are scheduled without a priority class
priorityClassName: ""

# -- enable server debug mode
# ref: https://docs.prefect.io/v3/develop/settings-ref#base-path
Copy link
Contributor Author

Choose a reason for hiding this comment

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

added additional docs ref's for specific server environment variables. Also removed double ##

@jamiezieziula jamiezieziula marked this pull request as ready for review March 3, 2025 16:24
@jamiezieziula jamiezieziula requested a review from a team as a code owner March 3, 2025 16:24
Copy link
Contributor

@mitchnielsen mitchnielsen left a comment

Choose a reason for hiding this comment

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

👌🏼

@jamiezieziula jamiezieziula merged commit 9c16f9b into main Mar 3, 2025
13 checks passed
@jamiezieziula jamiezieziula deleted the add-support-for-api-base-path branch March 3, 2025 18:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An improvement of an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants