Preserve Module-defined Timeout Values (Fixes #1515)

[Aarush289]

Proposed change

This PR fixes an issue where the timeout field defined in modules was always being overwritten by the default CLI timeout value (e.g., 3.0), even when the user did not explicitly provide a timeout.
Behavior After Fix
Module-defined timeout is preserved when no CLI timeout is provided.
CLI-provided timeout overrides module value only when explicitly set (i.e., not equal to default 3.0).
Fixes #1515
Tested by changing timeout value of one of the modules :-

Type of change

• New core framework functionality
• Bugfix (non-breaking change that fixes an issue)
• Code refactoring without any functionality changes
• New or existing module/payload change
• Documentation/localization improvement
• Test coverage improvement
• Dependency upgrade
• Other improvement (best practice, cleanup, optimization, etc)

Checklist

• I've followed the contributing guidelines
• I've digitally signed all my commits in this PR
• I've run make pre-commit and confirm it didn't generate any warnings/changes
• I've run make test and I confirm all tests passed locally
• I've added/updated any relevant documentation in the docs/ folder
• I've linked this PR with an open issue
• I've tested and verified that my code works as intended and resolves the issue as described
• I've attached screenshots demonstrating that my code works as intended (if applicable)
• I've checked all other open PRs to avoid submitting duplicate work
• I confirm that the code and comments in this PR are not direct unreviewed outputs of AI
• I confirm that I am the Sole Responsible Author for every line of code, comment, and design decision

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: c5393408-c546-49e7-b2d9-e6a0e10727b2

📥 Commits

Reviewing files that changed from the base of the PR and between a8c9efb and cc18743.

📒 Files selected for processing (1)

• nettacker/core/template.py

🚧 Files skipped from review as they are similar to previous changes (1)

• nettacker/core/template.py

---

Summary by CodeRabbit

• Bug Fixes
  • Improved template parameter processing for more accurate application of configuration values.
  • Refined timeout handling so user-specified timeout settings are preserved when applying templates.

Walkthrough

TemplateLoader.parse in nettacker/core/template.py now conditionally overrides YAML values from module_inputs: non-"timeout" keys are replaced only when module_inputs[key] is truthy. For "timeout", the YAML value is preserved unless abs(module_inputs["timeout"] - Config.settings.timeout) > 1e-9.

Changes

Cohort / File(s)	Summary
Template parsing / timeout handling nettacker/core/template.py	Adjusted TemplateLoader.parse to treat "timeout" specially: only replace YAML timeout when the provided module_inputs["timeout"] differs from Config.settings.timeout by more than 1e-9; other keys are replaced only when truthy.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Suggested labels

bug fixed

Suggested reviewers

• arkid15r
• securestep9

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: preserving module-defined timeout values when CLI timeout is not explicitly provided, and references the fixed issue (#1515).
Description check	✅ Passed	The description clearly explains the issue being fixed, the behavior after the fix, links to the related issue (#1515), and includes test evidence demonstrating the fix works as intended.
Linked Issues check	✅ Passed	The code change successfully addresses issue #1515 by implementing conditional logic that preserves module-defined timeout values when CLI timeout matches the default, only overriding when explicitly set by the user.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing the timeout override issue in TemplateLoader.parse; no unrelated modifications are present.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

nettacker/core/template.py (1)

21-21: Use the configured default timeout instead of a magic number.

Line 21 hardcodes 3.0; using Config.settings.timeout keeps this logic aligned with config if the default changes later.

Proposed fix

-                    elif key == "timeout" and abs(module_inputs[key] - 3.0) > 1e-9:
+                    elif key == "timeout" and abs(module_inputs[key] - Config.settings.timeout) > 1e-9:
                         module_content[key] = module_inputs[key]

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@nettacker/core/template.py` at line 21, Replace the hardcoded magic number
3.0 in the timeout comparison inside the template logic (the branch that checks
key == "timeout" and inspects module_inputs[key]) with the configured default
timeout value (Config.settings.timeout); i.e., use abs(module_inputs[key] -
Config.settings.timeout) > 1e-9 so the check follows the configured default and
not a literal 3.0.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@nettacker/core/template.py`:
- Around line 18-19: The current conditional in the template merge drops
explicit falsy overrides (0, False, "") because it tests truthiness; update the
condition in the merge logic that writes into module_content (referencing
module_inputs and module_content in template.py) to allow explicit falsy values
to override YAML: only skip the override when the key is "timeout" or the key is
absent/None. Concretely, replace the truthy check with a presence/None check so
keys present in module_inputs (even if 0/False/empty string) are copied into
module_content, while still excluding "timeout" as before.

---

Nitpick comments:
In `@nettacker/core/template.py`:
- Line 21: Replace the hardcoded magic number 3.0 in the timeout comparison
inside the template logic (the branch that checks key == "timeout" and inspects
module_inputs[key]) with the configured default timeout value
(Config.settings.timeout); i.e., use abs(module_inputs[key] -
Config.settings.timeout) > 1e-9 so the check follows the configured default and
not a literal 3.0.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: cdb8e946-e782-467e-a1fd-6e816da7e451

📥 Commits

Reviewing files that changed from the base of the PR and between 06c9bdf and a8c9efb.

📒 Files selected for processing (1)

• nettacker/core/template.py