fix(press-job): Workflow kv object doesn't support default#6425
Conversation
|
@tanmoysrt, thanks for the contribution, but we do not accept pull requests on a master. Please close this PR and raise PR on an develop branch. |
|
🎉 This PR is included in version 0.31.7 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
|
| Filename | Overview |
|---|---|
| press/press/doctype/press_job/jobs/create_server.py | Fixes the kv.get() call that incorrectly passed a default argument the interface doesn't support, but introduces an operator-precedence bug on line 128: or 0 + 1 evaluates as or (0 + 1), freezing the retry counter at 1. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[create_volume_from_snapshot called] --> B{data_disk_snapshot exists?}
B -- No --> C[return early]
B -- Yes --> D{snapshot_volume_id already set?}
D -- Yes --> C
D -- No --> E[Read volume_creation_attempts from kv]
E --> F{attempts OR 0 >= max_retries?}
F -- Yes --> G[raise Exception]
F -- No --> H[create_data_disk_volume_from_snapshot]
H --> I{is_created?}
I -- Yes --> C
I -- No --> J["kv.set(attempts, (attempts OR 0) + 1)"]
J --> K[defer_current_task]
style J fill:#f96,color:#000
style G fill:#f66,color:#fff
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
press/press/doctype/press_job/jobs/create_server.py:128
Operator precedence bug: `+` binds tighter than `or`, so this line evaluates as `self.kv.get(...) or (0 + 1)` — i.e. `self.kv.get(...) or 1`. After the first attempt stores `1`, every subsequent call hits `1 or 1 → 1` and the counter is permanently stuck at `1`. The `>= max_retries` guard (default 6) will therefore never fire after the first retry, leaving the job retrying indefinitely.
```suggestion
self.kv.set("volume_creation_attempts", (self.kv.get("volume_creation_attempts") or 0) + 1)
```
Reviews (1): Last reviewed commit: "fix(press-job): Workflow kv object doesn..." | Re-trigger Greptile
| return | ||
|
|
||
| self.kv.set("volume_creation_attempts", self.kv.get("volume_creation_attempts", 0) + 1) | ||
| self.kv.set("volume_creation_attempts", self.kv.get("volume_creation_attempts") or 0 + 1) |
There was a problem hiding this comment.
Operator precedence bug:
+ binds tighter than or, so this line evaluates as self.kv.get(...) or (0 + 1) — i.e. self.kv.get(...) or 1. After the first attempt stores 1, every subsequent call hits 1 or 1 → 1 and the counter is permanently stuck at 1. The >= max_retries guard (default 6) will therefore never fire after the first retry, leaving the job retrying indefinitely.
| self.kv.set("volume_creation_attempts", self.kv.get("volume_creation_attempts") or 0 + 1) | |
| self.kv.set("volume_creation_attempts", (self.kv.get("volume_creation_attempts") or 0) + 1) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: press/press/doctype/press_job/jobs/create_server.py
Line: 128
Comment:
Operator precedence bug: `+` binds tighter than `or`, so this line evaluates as `self.kv.get(...) or (0 + 1)` — i.e. `self.kv.get(...) or 1`. After the first attempt stores `1`, every subsequent call hits `1 or 1 → 1` and the counter is permanently stuck at `1`. The `>= max_retries` guard (default 6) will therefore never fire after the first retry, leaving the job retrying indefinitely.
```suggestion
self.kv.set("volume_creation_attempts", (self.kv.get("volume_creation_attempts") or 0) + 1)
```
How can I resolve this? If you propose a fix, please make it concise.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #6425 +/- ##
===========================================
+ Coverage 49.40% 59.71% +10.31%
===========================================
Files 940 110 -830
Lines 78104 17348 -60756
Branches 355 355
===========================================
- Hits 38588 10360 -28228
+ Misses 39493 6965 -32528
Partials 23 23
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
(cherry picked from commit 16d1b6c)