Skip to content
This repository was archived by the owner on Jun 11, 2025. It is now read-only.

KLO-177 : Add statefull field in nodepool entity#281

Merged
nxtcoder17 merged 2 commits into
mainfrom
update/nodepool-state
Feb 29, 2024
Merged

KLO-177 : Add statefull field in nodepool entity#281
nxtcoder17 merged 2 commits into
mainfrom
update/nodepool-state

Conversation

@nxtcoder19
Copy link
Copy Markdown
Contributor

No description provided.

@nxtcoder19 nxtcoder19 changed the title Add statefull field in nodepool entity KLO-177 : Add statefull field in nodepool entity Feb 29, 2024
Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

PR Type: Enhancement

PR Summary: The pull request introduces several enhancements and structural changes to the node pool entity and related models within the application. It adds new fields such as ImageID, ImageSSHUsername, and IsStateful to various structs, indicating an expansion in the configuration capabilities for AWS node pools and the introduction of stateful node pools. Additionally, it refactors the cloud provider constants to include Digitalocean and removes the unused InfrastuctureAsCode struct. The changes also include updates to error handling and simplification of resolver structures.

Decision: Comment

📝 Type: 'Enhancement' - not supported yet.
  • Sourcery currently only approves 'Typo fix' PRs.
✅ Issue addressed: this change correctly addresses the issue or implements the desired feature.
No details provided.
📝 Complexity: the changes are too large or complex for Sourcery to approve.
  • Unsupported files: the diff contains files that Sourcery does not currently support during reviews.
  • Big diff: the diff is too large to approve with confidence.

General suggestions:

  • Consider the implications of tightly coupling model structures to specific cloud providers. It might be beneficial to explore more generic approaches that can accommodate future expansions or changes to other cloud providers without significant refactoring.
  • Review the naming conventions used for new fields and structs to ensure consistency and clarity across the codebase. This includes considering the use of full names (e.g., DigitalOcean instead of Digitalocean) for better readability.
  • Evaluate the error handling strategy for the new stateful property in node pools to ensure that it aligns with the application's overall error management practices and provides clear feedback to the user.

Thanks for using Sourcery. We offer it for free for open source projects and would be very grateful if you could help us grow. If you like it, would you consider sharing Sourcery on your favourite social media? ✨

Share Sourcery

Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

Comment on lines +160 to +161
ImageID string `json:"imageId"`
ImageSSHUsername string `json:"imageSSHUsername"`
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

suggestion (llm): Adding ImageID and ImageSSHUsername directly to GithubComKloudliteOperatorApisClustersV1AWSNodePoolConfigIn struct increases the coupling with AWS-specific configurations. Consider abstracting these into a nested struct to maintain cloud provider agnosticism and facilitate future extensions for other cloud providers.

Comment on lines +350 to +351
KeyAWSVPCId func(childComplexity int) int
KeyAWSVPCPublicSubnets func(childComplexity int) int
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🚨 security (llm): Introduction of AWS VPC ID and Public Subnets as part of the ClusterOutput might not be a security concern directly unless these are being populated with sensitive or hardcoded values elsewhere in the code. Ensure that any use of these fields does not inadvertently expose sensitive information.

@nxtcoder17 nxtcoder17 merged commit 451fa17 into main Feb 29, 2024
@nxtcoder17 nxtcoder17 deleted the update/nodepool-state branch February 29, 2024 12:21
abdheshnayak pushed a commit that referenced this pull request Nov 5, 2024
KLO-177 : Add statefull field in nodepool entity
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants