Skip to content

Commit 8cc6f73

Browse files
prkhrkatprkhrkat
committed
review commnets
1 parent 7a36320 commit 8cc6f73

File tree

2 files changed

+66
-27
lines changed

2 files changed

+66
-27
lines changed

‎api/restHandler/app/pipeline/configure/BuildPipelineRestHandler.go

Lines changed: 66 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,12 @@ func (handler *PipelineConfigRestHandlerImpl) CreateCiConfig(w http.ResponseWrit
144144
}
145145

146146
createResp, err := handler.pipelineBuilder.CreateCiPipeline(&createRequest)
147-
handler.handleServiceError(w, err, createResp, "create ci config", createRequest)
147+
if err != nil {
148+
handler.Logger.Errorw("service err, create", "err", err, "create request", createRequest)
149+
common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
150+
return
151+
}
152+
common.WriteJsonResp(w, err, createResp, http.StatusOK)
148153
}
149154

150155
func (handler *PipelineConfigRestHandlerImpl) UpdateCiTemplate(w http.ResponseWriter, r *http.Request) {
@@ -172,7 +177,12 @@ func (handler *PipelineConfigRestHandlerImpl) UpdateCiTemplate(w http.ResponseWr
172177
}
173178

174179
createResp, err := handler.pipelineBuilder.UpdateCiTemplate(&configRequest)
175-
handler.handleServiceError(w, err, createResp, "UpdateCiTemplate", configRequest)
180+
if err != nil {
181+
handler.Logger.Errorw("service err", "err", err, "context", "UpdateCiTemplate", "data", configRequest)
182+
common.WriteJsonResp(w, err, createResp, http.StatusInternalServerError)
183+
return
184+
}
185+
common.WriteJsonResp(w, nil, createResp, http.StatusOK)
176186
}
177187

178188
func (handler *PipelineConfigRestHandlerImpl) UpdateBranchCiPipelinesWithRegex(w http.ResponseWriter, r *http.Request) {
@@ -218,7 +228,12 @@ func (handler *PipelineConfigRestHandlerImpl) UpdateBranchCiPipelinesWithRegex(w
218228

219229
//if include/exclude configured showAll will include excluded materials also in list, if not configured it will ignore this flag
220230
resp, err := handler.ciHandler.FetchMaterialsByPipelineId(patchRequest.Id, false)
221-
handler.handleServiceError(w, err, resp, "FetchMaterials", map[string]interface{}{"pipelineId": patchRequest.Id})
231+
if err != nil {
232+
handler.Logger.Errorw("service err", "err", err, "context", "FetchMaterials", "data", map[string]interface{}{"pipelineId": patchRequest.Id})
233+
common.WriteJsonResp(w, err, resp, http.StatusInternalServerError)
234+
return
235+
}
236+
common.WriteJsonResp(w, nil, resp, http.StatusOK)
222237
}
223238

224239
func (handler *PipelineConfigRestHandlerImpl) parseSourceChangeRequest(w http.ResponseWriter, r *http.Request) (*bean.CiMaterialPatchRequest, int32, error) {
@@ -525,7 +540,12 @@ func (handler *PipelineConfigRestHandlerImpl) GetCiPipeline(w http.ResponseWrite
525540
}
526541

527542
ciConf, err := handler.pipelineBuilder.GetCiPipelineRespResolved(appId)
528-
handler.handleServiceError(w, err, ciConf, "GetCiPipelineRespResolved", map[string]interface{}{"appId": appId})
543+
if err != nil {
544+
handler.Logger.Errorw("service err", "err", err, "context", "GetCiPipelineRespResolved", "data", map[string]interface{}{"appId": appId})
545+
common.WriteJsonResp(w, err, ciConf, http.StatusInternalServerError)
546+
return
547+
}
548+
common.WriteJsonResp(w, nil, ciConf, http.StatusOK)
529549
}
530550

531551
func (handler *PipelineConfigRestHandlerImpl) GetExternalCi(w http.ResponseWriter, r *http.Request) {
@@ -542,7 +562,12 @@ func (handler *PipelineConfigRestHandlerImpl) GetExternalCi(w http.ResponseWrite
542562
}
543563

544564
ciConf, err := handler.pipelineBuilder.GetExternalCi(appId)
545-
handler.handleServiceError(w, err, ciConf, "GetExternalCi", map[string]interface{}{"appId": appId})
565+
if err != nil {
566+
handler.Logger.Errorw("service err", "err", err, "context", "GetExternalCi", "data", map[string]interface{}{"appId": appId})
567+
common.WriteJsonResp(w, err, ciConf, http.StatusInternalServerError)
568+
return
569+
}
570+
common.WriteJsonResp(w, nil, ciConf, http.StatusOK)
546571
}
547572

548573
func (handler *PipelineConfigRestHandlerImpl) GetExternalCiById(w http.ResponseWriter, r *http.Request) {
@@ -564,7 +589,12 @@ func (handler *PipelineConfigRestHandlerImpl) GetExternalCiById(w http.ResponseW
564589
}
565590

566591
ciConf, err := handler.pipelineBuilder.GetExternalCiById(appId, externalCiId)
567-
handler.handleServiceError(w, err, ciConf, "GetExternalCiById", map[string]interface{}{"appId": appId, "externalCiId": externalCiId})
592+
if err != nil {
593+
handler.Logger.Errorw("service err", "err", err, "context", "GetExternalCiById", "data", map[string]interface{}{"appId": appId, "externalCiId": externalCiId})
594+
common.WriteJsonResp(w, err, ciConf, http.StatusInternalServerError)
595+
return
596+
}
597+
common.WriteJsonResp(w, nil, ciConf, http.StatusOK)
568598
}
569599

570600
func (handler *PipelineConfigRestHandlerImpl) validateCiTriggerRBAC(token string, ciPipelineId, triggerEnvironmentId int) error {
@@ -719,7 +749,12 @@ func (handler *PipelineConfigRestHandlerImpl) FetchMaterials(w http.ResponseWrit
719749
}
720750

721751
resp, err := handler.ciHandler.FetchMaterialsByPipelineId(pipelineId, showAll)
722-
handler.handleServiceError(w, err, resp, "FetchMaterials", map[string]interface{}{"pipelineId": pipelineId})
752+
if err != nil {
753+
handler.Logger.Errorw("service err", "err", err, "context", "FetchMaterials", "data", map[string]interface{}{"pipelineId": pipelineId})
754+
common.WriteJsonResp(w, err, resp, http.StatusInternalServerError)
755+
return
756+
}
757+
common.WriteJsonResp(w, nil, resp, http.StatusOK)
723758
}
724759

725760
func (handler *PipelineConfigRestHandlerImpl) FetchMaterialsByMaterialId(w http.ResponseWriter, r *http.Request) {
@@ -758,7 +793,12 @@ func (handler *PipelineConfigRestHandlerImpl) FetchMaterialsByMaterialId(w http.
758793
}
759794

760795
resp, err := handler.ciHandler.FetchMaterialsByPipelineIdAndGitMaterialId(pipelineId, gitMaterialId, showAll)
761-
handler.handleServiceError(w, err, resp, "FetchMaterialsByMaterialId", map[string]interface{}{"pipelineId": pipelineId, "gitMaterialId": gitMaterialId})
796+
if err != nil {
797+
handler.Logger.Errorw("service err", "err", err, "context", "FetchMaterialsByMaterialId", "data", map[string]interface{}{"pipelineId": pipelineId, "gitMaterialId": gitMaterialId})
798+
common.WriteJsonResp(w, err, resp, http.StatusInternalServerError)
799+
return
800+
}
801+
common.WriteJsonResp(w, nil, resp, http.StatusOK)
762802
}
763803

764804
func (handler *PipelineConfigRestHandlerImpl) RefreshMaterials(w http.ResponseWriter, r *http.Request) {
@@ -789,7 +829,12 @@ func (handler *PipelineConfigRestHandlerImpl) RefreshMaterials(w http.ResponseWr
789829
}
790830

791831
resp, err := handler.ciHandler.RefreshMaterialByCiPipelineMaterialId(material.Id)
792-
handler.handleServiceError(w, err, resp, "RefreshMaterials", map[string]interface{}{"gitMaterialId": gitMaterialId})
832+
if err != nil {
833+
handler.Logger.Errorw("service err", "err", err, "context", "RefreshMaterials", "data", map[string]interface{}{"gitMaterialId": gitMaterialId})
834+
common.WriteJsonResp(w, err, resp, http.StatusInternalServerError)
835+
return
836+
}
837+
common.WriteJsonResp(w, nil, resp, http.StatusOK)
793838
}
794839

795840
func (handler *PipelineConfigRestHandlerImpl) GetCiPipelineMin(w http.ResponseWriter, r *http.Request) {
@@ -826,7 +871,12 @@ func (handler *PipelineConfigRestHandlerImpl) GetCiPipelineMin(w http.ResponseWr
826871
}
827872

828873
ciPipelines, err := handler.pipelineBuilder.GetCiPipelineMin(appId, envIds)
829-
handler.handleServiceError(w, err, ciPipelines, "GetCiPipelineMin", map[string]interface{}{"appId": appId})
874+
if err != nil {
875+
handler.Logger.Errorw("service err", "err", err, "context", "GetCiPipelineMin", "data", map[string]interface{}{"appId": appId})
876+
common.WriteJsonResp(w, err, ciPipelines, http.StatusInternalServerError)
877+
return
878+
}
879+
common.WriteJsonResp(w, nil, ciPipelines, http.StatusOK)
830880
}
831881

832882
func (handler *PipelineConfigRestHandlerImpl) DownloadCiWorkflowArtifacts(w http.ResponseWriter, r *http.Request) {
@@ -921,7 +971,12 @@ func (handler *PipelineConfigRestHandlerImpl) GetHistoricBuildLogs(w http.Respon
921971
}
922972

923973
resp, err := handler.ciHandler.GetHistoricBuildLogs(workflowId, nil)
924-
handler.handleServiceError(w, err, resp, "GetHistoricBuildLogs", map[string]interface{}{"pipelineId": pipelineId, "workflowId": workflowId})
974+
if err != nil {
975+
handler.Logger.Errorw("service err", "err", err, "context", "GetHistoricBuildLogs", "data", map[string]interface{}{"pipelineId": pipelineId, "workflowId": workflowId})
976+
common.WriteJsonResp(w, err, resp, http.StatusInternalServerError)
977+
return
978+
}
979+
common.WriteJsonResp(w, nil, resp, http.StatusOK)
925980
}
926981

927982
func (handler *PipelineConfigRestHandlerImpl) GetBuildHistory(w http.ResponseWriter, r *http.Request) {

‎api/restHandler/app/pipeline/configure/BuildPipelineRestHandlerHelper.go

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import (
2121
"fmt"
2222
"github.com/devtron-labs/devtron/api/restHandler/common"
2323
"github.com/devtron-labs/devtron/internal/sql/repository/pipelineConfig"
24-
"github.com/devtron-labs/devtron/internal/util"
2524
"github.com/devtron-labs/devtron/pkg/auth/authorisation/casbin"
2625
"github.com/devtron-labs/devtron/pkg/bean"
2726
"net/http"
@@ -122,21 +121,6 @@ func (handler *PipelineConfigRestHandlerImpl) getCiPipelineWithAuth(w http.Respo
122121
return ciPipeline, true
123122
}
124123

125-
// handleServiceError handles service errors and writes appropriate response
126-
func (handler *PipelineConfigRestHandlerImpl) handleServiceError(w http.ResponseWriter, err error, resp interface{}, logContext string, logData interface{}) {
127-
if err != nil {
128-
if util.IsErrNoRows(err) {
129-
err = &util.ApiError{Code: "404", HttpStatusCode: http.StatusNotFound, UserMessage: "no data found"}
130-
common.WriteJsonResp(w, err, nil, http.StatusOK)
131-
} else {
132-
handler.Logger.Errorw("service err", "err", err, "context", logContext, "data", logData)
133-
common.WriteJsonResp(w, err, resp, http.StatusInternalServerError)
134-
}
135-
return
136-
}
137-
common.WriteJsonResp(w, nil, resp, http.StatusOK)
138-
}
139-
140124
// getQueryParamBool gets a boolean query parameter from the request
141125
func (handler *PipelineConfigRestHandlerImpl) getQueryParamBool(r *http.Request, paramName string, defaultValue bool) bool {
142126
v := r.URL.Query()

0 commit comments

Comments
 (0)