-
Couldn't load subscription status.
- Fork 20
Added checks in the dashmate template before accessing node properties #724
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
Conversation
WalkthroughUpdates Jinja2 conditional expressions in a dashmate template to safely handle undefined Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes The changes follow a consistent pattern of adding defensive guards across a single template file with similar logic repeated for different properties. Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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 (2)
ansible/roles/dashmate/templates/dashmate.json.j2 (2)
139-139: Core debug logic looks good; minor spacing nit.The defensive guard correctly prevents undefined variable errors, and the OR logic allows both global and per-node debug settings.
Optional: Add a space before
%}for consistency with the rest of the template:- "enabled": {% if dashd_debug == 1 or (node is defined and node.get('dashd_debug', 0) == 1)%}true{% else %}false{% endif %}, + "enabled": {% if dashd_debug == 1 or (node is defined and node.get('dashd_debug', 0) == 1) %}true{% else %}false{% endif %},
435-435: Correct fallback logic for Tenderdash log level.The defensive guard prevents undefined variable errors, and the fallback chain correctly prioritizes node-specific settings over the global default.
Optional: The logic is slightly redundant since
node.get('tenderdash_debug', tenderdash_log_level)already provides the fallback. However, this is not incorrect and the current form is explicit and clear.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ansible/roles/dashmate/templates/dashmate.json.j2(4 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: dashpay/dash-network-deploy#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-06T08:09:00.292Z
Learning: Applies to ansible/roles/dashmate/tasks/*.yml : Use `is defined` checks and conditional execution for variables such as `dashmate_group_check`, `dashmate_user_check`, `dash_conf_stat`, `dash_conf_changed`, `logrotate_config_stat`, `dashmate_update`, `dashmate_start_all`, `dashmate_restart_all`, `dashmate_install_result`, and `template_result` in Ansible tasks to prevent undefined variable errors
📚 Learning: 2025-08-06T08:09:00.292Z
Learnt from: CR
PR: dashpay/dash-network-deploy#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-06T08:09:00.292Z
Learning: Applies to ansible/roles/dashmate/tasks/*.yml : Use `is defined` checks and conditional execution for variables such as `dashmate_group_check`, `dashmate_user_check`, `dash_conf_stat`, `dash_conf_changed`, `logrotate_config_stat`, `dashmate_update`, `dashmate_start_all`, `dashmate_restart_all`, `dashmate_install_result`, and `template_result` in Ansible tasks to prevent undefined variable errors
Applied to files:
ansible/roles/dashmate/templates/dashmate.json.j2
🔇 Additional comments (2)
ansible/roles/dashmate/templates/dashmate.json.j2 (2)
281-281: LGTM! Drive debug configuration is correctly guarded.The conditional logic consistently applies the
node is definedguard before checkingdrive_debug, preventing undefined variable errors while enabling debug features when requested.Also applies to: 316-317
441-442: Ignore fallback for node keys in dashmate.json.j2
The empty-string fallback fornode.node_key.idandnode.node_key.private_keyis intentional for non-validator nodes and deployment tests show no errors.Likely an incorrect or invalid review comment.
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.
LGTM
Problem
Deployment was failing with
AnsibleUndefinedVariable: 'node' is undefinederrors on certain HP masternodes (e.g., hp-masternode-3, 4, 6) that don't have thenodevariable defined in their inventory.Solution
Added
node is definedchecks in the dashmate template before accessing node properties:dashd_debugdrive_debugdrive_debugfor visualizer configTesting
nodevariable definednodeis undefinedSummary by CodeRabbit