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

manager: add custom annotations as param on create/update instances #128

Merged
merged 3 commits into from
Dec 13, 2022

Conversation

morpheu
Copy link
Member

@morpheu morpheu commented Dec 12, 2022

No description provided.

@morpheu morpheu requested a review from nettoclaudio December 12, 2022 21:20
Copy link
Member

@nettoclaudio nettoclaudio left a comment

Choose a reason for hiding this comment

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

Just requested small fixes but lgtm overall.

@@ -252,6 +262,43 @@ type CertificateData struct {
Key string `json:"key"`
}

func getAnnotations(params map[string]interface{}) (annotations map[string]string) {
Copy link
Member

Choose a reason for hiding this comment

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

There are a bunch of validations here. It'd better returning an error in this function in order to make the client notice what happened.

Copy link
Member Author

Choose a reason for hiding this comment

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

You're right. At first glance seems to be a first idea just try to ignore them but it's better return it to client

internal/pkg/rpaas/manager.go Outdated Show resolved Hide resolved
@morpheu morpheu merged commit b224357 into main Dec 13, 2022
@morpheu morpheu deleted the plan_param_map branch December 13, 2022 19:52
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