evalai-cli icon indicating copy to clipboard operation
evalai-cli copied to clipboard

Published 20 hours ago • verified publisher icon Cloud-CV

[refactor] Replace conditional ladder in requests utils

Open nikochiko opened this issue 5 years ago • 5 comments

As a Google Code-In task.

Changes proposed:

Replace conditional ladder in requests utils, with a single requests.request call.

nikochiko avatar Dec 30 '19 07:12 nikochiko

@vkartik97 @RishabhJain2018 Shall I explicitly remove support for PUT, PATCH, DELETE requests? I realized in the last comment that it accepts all methods and that could give unexpected results.

nikochiko avatar Dec 30 '19 14:12 nikochiko

I realized in the last comment that it accepts all methods and that could give unexpected results.

Can you please provide detailed information regarding this?

krtkvrm avatar Dec 30 '19 14:12 krtkvrm

@vkartik97 @Ram81 I have generalized the handling message for all errors. The behavior of the function is the same. Please review.

nikochiko avatar Jan 01 '20 08:01 nikochiko

@Ram81 @vkartik97 I have changed the handling mechanism a bit, but the behavior of the function is the same, and with the suggested additions.

  1. Replace the conditional ladder using a single requests.request call.
  2. Generalize all exceptions caused by requests into a single block.
  3. Use same template for all error messages. This will help keep it consistent.
  4. Shorten the code (78 lines ->23 lines)
  5. Remove multiple except blocks.

The efficiency and readability of the code has increased. Please review.

nikochiko avatar Jan 01 '20 09:01 nikochiko

@Ram81 Can I get an approval on the GCI page?

nikochiko avatar Jan 02 '20 04:01 nikochiko