Skip to content

Issue #238 - add upsert flag to 'argocd app create' command#245

Merged
alexmt merged 1 commit intoargoproj:masterfrom
alexmt:238-app-upsert
May 30, 2018
Merged

Issue #238 - add upsert flag to 'argocd app create' command#245
alexmt merged 1 commit intoargoproj:masterfrom
alexmt:238-app-upsert

Conversation

@alexmt
Copy link
Copy Markdown
Collaborator

@alexmt alexmt commented May 30, 2018

Closes: #238

@alexmt alexmt requested review from jessesuen and merenbach May 30, 2018 16:07
Copy link
Copy Markdown
Contributor

@merenbach merenbach left a comment

Choose a reason for hiding this comment

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

@alexmt looks great! One minor question: earlier we added a feature to not complain on app create when the spec was the same. Since it now sounds as though that wasn't the desired behavior from stakeholders—instead, an upsert behavior, as implemented here, was desired—does it make sense to maybe remove the spec comparison and error out on app creation with the same name, as we used to, unless upsert is specified?

@alexmt
Copy link
Copy Markdown
Collaborator Author

alexmt commented May 30, 2018

Spec comparison makes sense to me. I think it is convienient to notify user about spec difference unless upsert flag is set. @merenbach if you don't mind I would prefer to keep that check

@merenbach
Copy link
Copy Markdown
Contributor

@alexmt sounds good!

@alexmt alexmt merged commit 96c05ba into argoproj:master May 30, 2018
@alexmt alexmt deleted the 238-app-upsert branch May 30, 2018 20:49
@@ -68,6 +69,13 @@ func (s *Server) List(ctx context.Context, q *ApplicationQuery) (*appv1.Applicat

// Create creates an application
func (s *Server) Create(ctx context.Context, a *appv1.Application) (*appv1.Application, error) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we just change the RPC signature to ApplicationCreateRequest instead of Application? Maybe not for 0.4.4 but for 0.5.0? Storing into context hides the API capabilities.

leoluz pushed a commit to leoluz/argo-cd that referenced this pull request Sep 29, 2023
Signed-off-by: kshamajain99 <kshamajain99@gmail.com>
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