feat(trackers/spd): handle staff reservations - #1387
Conversation
|
Thanks for taking the time to contribute to this project. Upload Assistant is currently in a complete rewrite, and no new development is being conducted on this python source at this time. If you have come this far, please feel free to leave open, any pull requests regarding new sites being added to the source, as these can serve as the baseline for later conversion. If your pull request relates to a critical bug, this will be addressed in this code base, and a new release published as needed. If your pull request only addresses a quite minor bug, it is not likely to be addressed in this code base. Details for the new code base will follow at a later date. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds SPD.get_claims(meta) to query SpeedApp’s /api/reservation and match reservations against the edited SPD name; TRACKER_SETUP.get_torrent_claims now initializes the tracker and directly returns tracker_instance.get_claims(meta) for SPD when available. ChangesSPD Claims API Integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/trackers/SPD.py`:
- Around line 55-68: The reservation normalization must use the same
Unicode/ASCII-folding as edit_name() so API names match; update the
normalized_res_name logic to apply the same folding routine used by edit_name()
(or call that helper if one exists) before stripping non-alphanumerics and
lowercasing. Concretely, where normalized_res_name is computed from
reservation.get('name'), first pass res_name through the same
ASCII-fold/Unicode-normalize function used by edit_name() (or reuse edit_name's
helper), then run the re.sub(r'[^a-zA-Z0-9]', '', ...) and .lower() so
normalized_spd_name and normalized_res_name use identical normalization. Ensure
you only change the normalization step for normalized_res_name and keep the
existing type checks for reservation.
In `@src/trackersetup.py`:
- Around line 430-433: The SPD tracker must not treat verification failures as
"not claimed": change the SPD implementation (the SPD.get_claims method) to
return None for configuration/API/transport failures (only True for claimed,
False for explicitly not claimed), and keep trackersetup.py's branch (where
tracker_instance.get_claims is awaited) returning the raw result
(None/True/False) unchanged so the later logic that builds local_tracker_status
can distinguish None (verification failure) from False (explicitly not claimed)
when computing local_tracker_status['skipped'].
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: a821d05d-d527-4d9e-8c84-844b19dbceb6
📒 Files selected for processing (2)
src/trackers/SPD.pysrc/trackersetup.py
| if tracker.upper() == "SPD": | ||
| if hasattr(tracker_instance, 'get_claims'): | ||
| return await tracker_instance.get_claims(meta) | ||
| return None |
There was a problem hiding this comment.
Don't treat “claims check failed” as “not claimed”.
This branch returns SPD's raw boolean straight into the normal claims flow. In src/trackerstatus.py:94-105, that result is collapsed into local_tracker_status['skipped'] = bool(claimed), so the False returned by SPD.get_claims() on missing API keys, 403s, and transport errors becomes “safe to upload”. That makes the new staff-reservation guard fail open whenever the API cannot be verified.
🛡️ Suggested contract change
if tracker.upper() == "SPD":
if hasattr(tracker_instance, 'get_claims'):
- return await tracker_instance.get_claims(meta)
+ claimed = await tracker_instance.get_claims(meta)
+ if claimed is None:
+ meta['tracker_status'].setdefault(tracker, {})['skip_upload'] = True
+ return True
+ return claimed
return NoneAnd have SPD.get_claims() return None for configuration/API failures instead of False.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/trackersetup.py` around lines 430 - 433, The SPD tracker must not treat
verification failures as "not claimed": change the SPD implementation (the
SPD.get_claims method) to return None for configuration/API/transport failures
(only True for claimed, False for explicitly not claimed), and keep
trackersetup.py's branch (where tracker_instance.get_claims is awaited)
returning the raw result (None/True/False) unchanged so the later logic that
builds local_tracker_status can distinguish None (verification failure) from
False (explicitly not claimed) when computing local_tracker_status['skipped'].
1 participant
#1246
Summary by CodeRabbit