-
Notifications
You must be signed in to change notification settings - Fork 116
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
fix: Add default port when undefined #141
fix: Add default port when undefined #141
Conversation
WalkthroughThis pull request updates the Helm chart deployment templates for n8n by enhancing the retrieval logic for the container port configuration. In the templates for the webhook, worker, and main deployments, the configuration lookup is now wrapped with a Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
charts/n8n/templates/deployment.yaml (1)
68-68
: Refined Environment Variable FormattingThe minor formatting adjustment for the
env
declaration on line 68 (ensuring there is no extraneous whitespace) improves readability and consistency. Although it does not change functionality, it contributes to a cleaner template.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
charts/n8n/templates/deployment.webhook.yaml
(1 hunks)charts/n8n/templates/deployment.worker.yaml
(1 hunks)charts/n8n/templates/deployment.yaml
(2 hunks)
🔇 Additional comments (3)
charts/n8n/templates/deployment.worker.yaml (1)
71-71
: Enhanced Port Retrieval Fallback in Worker DeploymentThe updated container port retrieval now wraps
.Values.worker.config
in adefault (dict)
function. This prevents errors in cases where the configuration is undefined and ensures that the"port"
key lookup safely defaults to5678
if not provided.charts/n8n/templates/deployment.yaml (1)
77-77
: Robust Container Port Configuration in Main DeploymentBy wrapping
.Values.main.config
indefault (dict)
, the template safely handles cases where the configuration is missing. This ensures that the port lookup falls back to the specified default (5678
) without triggering template errors.charts/n8n/templates/deployment.webhook.yaml (1)
69-69
: Consistent Port Configuration for Webhook DeploymentThe change to use
{{ get (default (dict) .Values.webhook.config) "port" | default 5678 }}
introduces a safety net by defaulting to an empty dictionary if the webhook configuration is undefined. This change aligns the webhook template with the updates made in both the worker and main deployments, ensuring robust and error-free port resolution.
thank you for your contribution, the whole worker webhook part is not yet complete, hence the pre-release |
fixes error if no port is defined in .Values.worker.config and .Values.webhook.config, e.g.
Summary by CodeRabbit