- 
                Notifications
    You must be signed in to change notification settings 
- Fork 554
feat: Change cd deployment type #3332
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
Conversation
| resp, err := handler.pipelineBuilder.ChangePipelineDeploymentType(ctx, deploymentTypeChangeRequest) | ||
|  | ||
| if err != nil { | ||
| nErr := errors.New("failed to change deployment type with error msg: " + err.Error()) | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can use format printer
|  | ||
| func (handler PipelineConfigRestHandlerImpl) HandleTriggerDeploymentAfterTypeChange(w http.ResponseWriter, r *http.Request) { | ||
|  | ||
| // Auth check | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove this comment
| func (impl PipelineRepositoryImpl) UpdateCdPipeline(pipeline *Pipeline) error { | ||
| err := impl.dbConnection.Update(pipeline) | ||
| func (impl PipelineRepositoryImpl) SetDeploymentAppCreatedInPipeline(deploymentAppCreated bool, pipelineId int, userId int32) error { | ||
| query := "update pipeline set deployment_app_created=?, updated_on=?, updated_by=? where id=?;" | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
avoid writing native queries
| query := "update pipeline set deployment_app_created = ?, deployment_app_type = ?, " + | ||
| "updated_by = ?, updated_on = ?, deployment_app_delete_request = ? where id in (?);" | ||
| var pipeline *Pipeline | ||
| _, err := impl.dbConnection.Query(pipeline, query, deploymentAppCreated, deploymentAppType, userId, time.Now(), delete, pg.In(cdPipelineIdIncludes)) | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
check empty array for pg.In
| "updated_by = ?, updated_on = ?, deployment_app_delete_request = ? where id in (?);" | ||
| var pipeline *Pipeline | ||
| _, err := impl.dbConnection.Query(pipeline, query, deploymentAppType, userId, time.Now(), pg.In(cdPipelineIdIncludes)) | ||
| _, err := impl.dbConnection.Query(pipeline, query, deploymentAppType, userId, time.Now(), delete, pg.In(cdPipelineIdIncludes)) | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use isDeleted
|  | ||
| for _, pipeline := range pipelines { | ||
|  | ||
| artifactDetails, err := impl.RetrieveArtifactsByCDPipeline(pipeline, "DEPLOY") | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it possible to extract out from iteration
        
          
                pkg/pipeline/PipelineBuilder.go
              
                Outdated
          
        
      | } | ||
| _, err = impl.application.Get(ctx, req) | ||
| } | ||
| fmt.Println(err.Error() == "rpc error: code = NotFound desc = error getting application: applications.argoproj.io \"app-1-test-env\" not found") | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hardcoded
| SonarCloud Quality Gate failed.     
 
 | 








With this PR, two API is being introduced to change the deployment type for all cd pipelines in an environment and trigger the deploy for the same.
Fixes #https://dev.azure.com/DevtronLabs/Devtron/_workitems/edit/2870
Checklist: