Add GPU server zone restriction and clarify spot price handling at UpCloud#101
Conversation
|
Warning Review limit reached
Next review available in: 36 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughUpCloud pricing inventory now caches per-zone GPU availability, uses it to filter regular GPU plans, and emits spot pricing records for ChangesUpCloud pricing inventory
Estimated code review effort: 4 (Complex) | ~40 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/sc_crawler/vendors/_upcloud.py (2)
450-527: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
inventory_server_pricesandinventory_server_prices_spotare near-identical; extract a shared builder.The two functions differ only in the SPOT filter direction, the prefix strip, and the
allocationvalue. This duplication is exactly what allowed the ordering bug above to appear in only one of them. Consider factoring the shared loop/price-record construction into a helper parametrized by allocation.🤖 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/sc_crawler/vendors/_upcloud.py` around lines 450 - 527, The two inventory functions duplicate almost the same price-record building logic, so extract the shared loop into a helper and parameterize the small differences. Refactor `inventory_server_prices` and `inventory_server_prices_spot` around a common helper that handles iterating `prices["prices"]["zone"]`, GPU availability checks, and appending the price payload, while passing in the allocation and the SPOT/non-SPOT filter behavior plus the server plan normalization. Keep the existing public function names as thin wrappers calling the shared builder so the logic stays in one place.
41-49: 🩺 Stability & Availability | 🔵 Trivial | ⚖️ Poor tradeoff
cachierhas nostale_after, so GPU availability is cached to disk indefinitely across runs.
separate_files=Truepersists results between crawler invocations. Since GPU/passthrough availability changes over time, a region that gains or loses GPU capacity will keep returning the first cached response, silently skewing which GPU plans get emitted. Consider setting astale_afterwindow aligned with your crawl cadence.♻️ Suggested change
-@cachier(hash_func=jsoned_hash, separate_files=True) +@cachier(hash_func=jsoned_hash, separate_files=True, stale_after=timedelta(hours=1)) def _get_device_region_availability(region_id: str, device_type: str = "gpu") -> dict:🤖 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/sc_crawler/vendors/_upcloud.py` around lines 41 - 49, The _get_device_region_availability cache is persisted by cachier with separate_files=True, so GPU availability can stay stale across crawler runs. Update this function to use a finite cache lifetime by adding an appropriate stale_after window aligned with crawl frequency, so region availability is refreshed regularly. Keep the change local to _get_device_region_availability and preserve the existing jsoned_hash/separate_files behavior.
🤖 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/sc_crawler/vendors/_upcloud.py`:
- Around line 502-508: Move the SPOT- prefix stripping in the server plan
handling logic before the GPU availability check so spot GPU plans are evaluated
correctly. In the code path inside the UpCloud vendor logic, update the server
plan normalization around the SPOT/GPU checks so server_plan is cleaned first,
then use the normalized value for the startswith("GPU") test and the
gpu_region_availability lookup.
---
Nitpick comments:
In `@src/sc_crawler/vendors/_upcloud.py`:
- Around line 450-527: The two inventory functions duplicate almost the same
price-record building logic, so extract the shared loop into a helper and
parameterize the small differences. Refactor `inventory_server_prices` and
`inventory_server_prices_spot` around a common helper that handles iterating
`prices["prices"]["zone"]`, GPU availability checks, and appending the price
payload, while passing in the allocation and the SPOT/non-SPOT filter behavior
plus the server plan normalization. Keep the existing public function names as
thin wrappers calling the shared builder so the logic stays in one place.
- Around line 41-49: The _get_device_region_availability cache is persisted by
cachier with separate_files=True, so GPU availability can stay stale across
crawler runs. Update this function to use a finite cache lifetime by adding an
appropriate stale_after window aligned with crawl frequency, so region
availability is refreshed regularly. Keep the change local to
_get_device_region_availability and preserve the existing
jsoned_hash/separate_files behavior.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0ade9003-fe26-4bfa-843b-3032519a6ca7
📒 Files selected for processing (1)
src/sc_crawler/vendors/_upcloud.py
bridgend84
changed the title
2 participants
Overview
Add GPU server zone restriction and clarify spot price handling at
UpCloud.Required checks before merge
Mark all tasks that were completed with a checkmark. Unrelated tasks can be left unchecked.
PR status:
mainVersioning, changelog, and documentation:
CHANGELOG.mdpyproject.tomladd_vendor.mdis up-to-date (after schema changes or new learnings that generalizes well to future vendors)mkdocsrenders correctly in local preview and has been manually checked on new or updated functions/methodsDatabase:
sc-crawler pullwas tested on all vendors and recordsSummary by CodeRabbit