sodafoundation / strato

SODA Strato (Multi-cloud) project provides a cloud vendor agnostic data management for hybrid cloud, intercloud or intracloud. This project is renamed as 'Strato'
Apache License 2.0
234 stars 329 forks source link

Code changed to show correct status from postman when job does not exist #1344

Closed subi9 closed 2 years ago

subi9 commented 2 years ago

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespace from that line: kind bug fix

What this PR does / why we need it: While using postman api, when the get jobs is hit, job number which does not exist in the system throws a 500 internal server error message which is wrong. When a job does not exist in the system it will simply throw an 404 error and show message as "Job does not exist in the system" Which issue(s) this PR fixes:

Fixes #https://github.com/sodafoundation/multi-cloud/issues/938

Test Report Added?:

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespace from that line:

/kind TESTED

/kind NOT-TESTED

Test Report:

Screenshot from 2022-04-29 12-28-00 Screenshot from 2022-04-29 21-35-22 Screenshot from 2022-04-29 21-35-32 Screenshot from 2022-04-29 21-35-44

Special notes for your reviewer:

PravinRanjan10 commented 2 years ago

@kumarashit please review this

PravinRanjan10 commented 2 years ago

LGTM

kumarashit commented 2 years ago

Please check the CI

PravinRanjan10 commented 2 years ago

PR Accepted

codecov[bot] commented 2 years ago

Codecov Report

Merging #1344 (f2621b4) into master (4285082) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1344   +/-   ##
=======================================
  Coverage   48.12%   48.12%           
=======================================
  Files          10       10           
  Lines        1571     1571           
=======================================
  Hits          756      756           
  Misses        756      756           
  Partials       59       59           
PravinRanjan10 commented 2 years ago

@kumarashit plz review

subi9 commented 2 years ago

Hi Ashit,

We can change the "job not exist" message and store it in a variable but "invalid input to object id hex" is not generic so we can keep as it is.

Please let me know.

Thanks, Subinoy

On Mon, May 2, 2022 at 8:07 AM Ashit Kumar @.***> wrote:

@.**** commented on this pull request.

In api/pkg/dataflow/service.go https://github.com/sodafoundation/multi-cloud/pull/1344#discussion_r862563547 :

@@ -420,6 +423,11 @@ func (s APIService) GetJob(request restful.Request, response *restful.Response ctx := common.InitCtxWithAuthInfo(request) res, err := s.dataflowClient.GetJob(ctx, &dataflow.GetJobRequest{Id: id}) if err != nil {

  • if strings.Contains(err.Error(), "job not exist") || strings.Contains(err.Error(), "invalid input to ObjectIdHex") {

Is it possible to put these strings as var and refer them here? These seems to be generic which can be referred elsewhere too

— Reply to this email directly, view it on GitHub https://github.com/sodafoundation/multi-cloud/pull/1344#pullrequestreview-958680078, or unsubscribe https://github.com/notifications/unsubscribe-auth/APHP6DESFIBTVJ7TQ64FCI3VH45UZANCNFSM5UWRIVBQ . You are receiving this because you authored the thread.Message ID: @.***>