Skip to content

Conversation

@anujhydrabadi
Copy link
Collaborator

@anujhydrabadi anujhydrabadi commented Sep 25, 2025

Description

Related issues

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Checklist

  • I have created feat/bugfix branch out of develop branch
  • Code passes linting/formatting checks
  • Changes to resources have been tested in our dev environments
  • I have made corresponding changes to the documentation

Testing

Reviewer instructions

Summary by CodeRabbit

  • New Features
    • Relaxed configuration requirements for service deployments: ports, size, and health_checks are no longer mandatory at the top-level spec, allowing more flexible setups while preserving existing behavior for other fields.
    • Relaxed validation for stateful set volumes: ports, size, and health_checks are no longer required within volume configurations, simplifying initialization and enabling leaner configs.
    • Existing configurations remain valid; these changes only make previously required fields optional without altering overall structure or UI ordering.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 25, 2025

Walkthrough

Removed required validations for ports, size, and health_checks in deployment and statefulset facets, relaxing schema constraints at the specified locations without altering other structure or fields.

Changes

Cohort / File(s) Summary
Deployment spec required fields removal
modules/service/deployment/0.1/facets.yaml
Deleted top-level spec.required entries for ports, size, and health_checks; x-ui-order unchanged.
StatefulSet volumes required fields removal
modules/service/statefulset/0.1/facets.yaml
Removed volumes.required entries for ports, size, and health_checks within spec, making them optional.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant User as Config Author
  participant Tool as Facets Validator
  participant Schema as Deployment/StatefulSet Facets

  User->>Tool: Submit service config
  Tool->>Schema: Validate against facets.yaml
  Note right of Schema: Required fields updated<br/>(ports, size, health_checks no longer mandatory<br/>at specified locations)
  alt Validation passes with optional fields absent
    Schema-->>Tool: OK
    Tool-->>User: Accept configuration
  else Other required constraints fail
    Schema-->>Tool: Error details
    Tool-->>User: Reject with validation errors
  end
Loading

Possibly related PRs

  • Update facets.yaml #480: Adjusts health_checks required entries (startup_*), intersecting with this PR’s health_checks validation changes in deployment facets.
  • [SYNC] Develop to Master #383: Modifies deployment and statefulset facets touching ports and health_checks, overlapping the same schema areas.

Suggested reviewers

  • rr0hit
  • vishnukv-facets
  • ishaankalra
  • pramodh-ayyappan

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The pull request description contains only the template headings without any actual summary of changes, references to related issues, type of change selection, checklist completion, testing details, or reviewer guidance, leaving all required sections unfilled. Fill out the description by summarizing the changes made, linking related issues, selecting the type of change, completing the checklist, and providing testing details and reviewer instructions as specified in the template.
Title Check ❓ Inconclusive The pull request title uses a vague placeholder “etc” and does not clearly specify which fields or facets are affected, making it difficult for readers to immediately understand the scope and intent of the change. Please revise the title to explicitly name the fields and context modified, for example: “Make ports, size, and healthChecks optional in service deployment and StatefulSet facets.”
✅ Passed checks (1 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch ports-not-required

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7a6d9f6 and 816dc61.

📒 Files selected for processing (2)
  • modules/service/deployment/0.1/facets.yaml (0 hunks)
  • modules/service/statefulset/0.1/facets.yaml (0 hunks)
💤 Files with no reviewable changes (2)
  • modules/service/statefulset/0.1/facets.yaml
  • modules/service/deployment/0.1/facets.yaml

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

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