Skip to content

Conversation

@moshemorad
Copy link
Contributor

No description provided.

@moshemorad moshemorad requested a review from arikalon1 December 17, 2025 19:15
@coderabbitai
Copy link

coderabbitai bot commented Dec 17, 2025

Walkthrough

The deleted field in the Info class of the Helm release model was changed from a required string parameter to an optional string parameter with an empty string default value, modifying initialization behavior.

Changes

Cohort / File(s) Summary
Helm release model field update
src/robusta/core/model/helm_release.py
Changed Info.deleted field signature from required (deleted: str) to optional with default value (deleted: str = ""), allowing initialization without explicitly providing this parameter

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

  • Specific areas for attention:
    • Verify this field is not relied upon as a required parameter elsewhere in the codebase
    • Confirm that defaulting to an empty string aligns with intended behavior for Helm release information handling
    • Check if any serialization/deserialization logic expects this field to always be present

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description check ❓ Inconclusive No description was provided by the author, making it impossible to evaluate whether it relates to the changeset. Add a pull request description explaining the motivation and impact of making the deleted field optional in HelmRelease.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'HelmRelease supprt missing info.deleted field' directly describes the main change: making the deleted field optional in the HelmRelease Info model.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch ROB-2828-discovery-failed-to-parse-helm-release

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

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

@moshemorad moshemorad enabled auto-merge (squash) December 17, 2025 19:21
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/robusta/core/model/helm_release.py (1)

24-24: Use Optional[str] = None for consistency with other optional fields in the class.

The fix correctly prevents parsing failures when deleted is missing from Helm release data. However, the current pattern with str = "" is inconsistent with other optional fields in the Info class (description and notes both use Optional[str]).

For consistency and clarity, consider:

-    deleted: str = ""
+    deleted: Optional[str] = None

This aligns with the class's existing pattern for optional string fields and makes it explicit that the field can be absent.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5d8742b and 4e5badd.

📒 Files selected for processing (1)
  • src/robusta/core/model/helm_release.py (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: run_tests
  • GitHub Check: run_tests
  • GitHub Check: Deploy docs

@moshemorad moshemorad merged commit a91ecd5 into master Dec 18, 2025
12 of 16 checks passed
@moshemorad moshemorad deleted the ROB-2828-discovery-failed-to-parse-helm-release branch December 18, 2025 10:33
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.

3 participants