Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clusterresources refs for other clusters #625

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

tengu-alt
Copy link
Collaborator

No description provided.

@tengu-alt tengu-alt added the refactor Code improvements or refactorings label Nov 16, 2023
@tengu-alt tengu-alt requested a review from ribaraka November 16, 2023 09:28
@tengu-alt tengu-alt self-assigned this Nov 16, 2023
@tengu-alt tengu-alt force-pushed the clusterresources-refs-for-other-clusters branch from 181a49e to e2755e7 Compare November 22, 2023 14:31
Comment on lines 42 to 44
type FirewallRuleSpec struct {
ClusterID string `json:"clusterId"`
Type string `json:"type"`
Type string `json:"type"`
}
Copy link
Contributor

Choose a reason for hiding this comment

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

How about we abandon this generic struct with only one field? There is also another one called VPCPeeringSpec

Comment on lines 29 to 31
type ClusterBackupSpec struct {
ClusterID string `json:"clusterId"`
ClusterKind string `json:"clusterKind"`
}
Copy link
Contributor

Choose a reason for hiding this comment

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

ClusterKind you can move simply into the Status and thus you will remove and simplify the code that calculates the appropriate Kind.
Another consideration is whether triggering a backup through a cluster spec enhances the user experience. Need to ask what others think.

}

type PeeringStatus struct {
ID string `json:"id,omitempty"`
StatusCode string `json:"statusCode,omitempty"`
Name string `json:"name,omitempty"`
FailureReason string `json:"failureReason,omitempty"`
CDCID string `json:"cdcid,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

camel case in json

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

@ribaraka
Copy link
Contributor

AWSEndpointServicePrincipal is missing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Code improvements or refactorings
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants