Skip to content

fix(rollback): use == instead of is for boolean check in execute_roll…#1207

Merged
paigerube14 merged 4 commits intokrkn-chaos:mainfrom
NETIZEN-11:fix/rollback-bugs
Mar 30, 2026
Merged

fix(rollback): use == instead of is for boolean check in execute_roll…#1207
paigerube14 merged 4 commits intokrkn-chaos:mainfrom
NETIZEN-11:fix/rollback-bugs

Conversation

@NETIZEN-11
Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes an incorrect boolean check in execute_rollback_version_files().

Type of change

  • Refactor
  • New feature
  • Bug fix
  • Optimization

Description

This PR fixes a bug caused by improper boolean comparison in
execute_rollback_version_files().

Previously, the condition used identity comparison (is False) on a boolean:
RollbackConfig().auto is False.

This is unreliable because identity comparison should not be used for boolean values.
It could result in unintended behavior where auto-rollback executes even when disabled.

The fix replaces it with a proper boolean check:
if not RollbackConfig().auto

Related Tickets & Documents

  • Related Issue #: N/A
  • Closes #: N/A

Documentation

  • Is documentation needed for this update?

Related Documentation PR (if applicable)

N/A

Checklist before requesting a review

  • I have performed a self-review of my code by running krkn and specific scenario

REQUIRED:

Description of combination of tests performed and output of run

Tested the rollback functionality manually by toggling the auto flag in configuration:

  • When auto = False, rollback is correctly skipped.
  • When auto = True, rollback executes as expected.
python run_kraken.py
Rollback execution skipped when auto=False
Rollback executed successfully when auto=True

@qodo-code-review
Copy link
Copy Markdown

Review Summary by Qodo

Fix boolean comparison in rollback handler

🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Replace identity comparison (is False) with equality comparison (== False)
• Fix unreliable boolean check in execute_rollback_version_files() function
• Add Apache License 2.0 header to file
• Fix indentation inconsistency in conditional block
Diagram
flowchart LR
  A["Identity comparison<br/>is False"] -->|"Replace with"| B["Equality comparison<br/>== False"]
  B -->|"Result"| C["Reliable boolean check<br/>in execute_rollback_version_files"]
Loading

Grey Divider

File Changes

1. krkn/rollback/handler.py 🐞 Bug fix +19/-3

Fix boolean comparison and add license header

• Added Apache License 2.0 copyright header at file start
• Changed boolean comparison from is False to == False in execute_rollback_version_files()
 function
• Fixed indentation of logger.warning and return statements (4 spaces to proper alignment)
• Maintains same logic while using proper boolean comparison operator

krkn/rollback/handler.py


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown

qodo-code-review bot commented Mar 27, 2026

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 📐 Spec deviations (0)

Grey Divider


Action required

1. False equality regression 🐞 Bug ✓ Correctness
Description
execute_rollback_version_files now uses RollbackConfig().auto == False, which can fail to treat
other falsey values (e.g., None/"") as disabled and may allow rollback execution when it should be
skipped. This also introduces a flake8 E712-style violation (comparison to False) based on the
repo’s flake8 configuration.
Code

krkn/rollback/handler.py[R150-152]

+    if not ignore_auto_rollback_config and RollbackConfig().auto == False:
+        logger.warning(f"Auto rollback is disabled, skipping execution for run_uuid={run_uuid or '*'}, scenario_type={scenario_type or '*'}")
+        return
Evidence
The PR introduces == False in the rollback gating condition. RollbackConfig.auto is assigned
without type validation/coercion, so .auto can be non-bool; == False only matches literal False
(unlike not ..., which correctly handles other falsey values). Additionally, the repo has flake8
configured and does not ignore the rule family that flags == False comparisons.

krkn/rollback/handler.py[150-152]
krkn/rollback/config.py[64-81]
setup.cfg[40-42]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`execute_rollback_version_files()` currently gates auto-rollback with `RollbackConfig().auto == False`. Because `RollbackConfig.auto` accepts any type (no validation/coercion), this check can mis-handle falsey non-bool values (e.g., `None`, `""`) and it’s also a flake8 E712-style pattern.

## Issue Context
- Rollback is a critical operation; the config gate should be robust for any falsey value.
- The PR description indicates the intended fix was a proper boolean check.

## Fix Focus Areas
- Replace the condition with an idiomatic falsey check, e.g.:
 - `if not ignore_auto_rollback_config and not RollbackConfig().auto:`
 - (Optionally) enforce boolean type in `RollbackConfig.auto` setter (e.g., validate `isinstance(value, bool)` or coerce via `bool(value)` depending on desired strictness).

- krkn/rollback/handler.py[150-152]
- krkn/rollback/config.py[67-81]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

…back_version_files

Signed-off-by: Nitesh <nitesh@example.com>
Copy link
Copy Markdown
Collaborator

@paigerube14 paigerube14 left a comment

Choose a reason for hiding this comment

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

/lgtm

@paigerube14 paigerube14 merged commit 8c57b09 into krkn-chaos:main Mar 30, 2026
1 of 5 checks passed
paigerube14 added a commit to paigerube14/kraken that referenced this pull request Mar 31, 2026
krkn-chaos#1207)

* fix(rollback): use == instead of is for boolean check in execute_rollback_version_files

Signed-off-by: Nitesh <nitesh@example.com>

* fix(rollback): fix indentation in execute_rollback_version_files and add license header

Signed-off-by: Nitesh <nitesh@example.com>

---------

Signed-off-by: Nitesh <nitesh@example.com>
Co-authored-by: Nitesh <nitesh@example.com>
Co-authored-by: Paige Patton <64206430+paigerube14@users.noreply.github.com>
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.

2 participants