search

Cloud-CV / evalai-cli

:cloud: :rocket: Official EvalAI Command Line Tool
https://cli.eval.ai
BSD 3-Clause "New" or "Revised" License
55 stars 63 forks source link

[refactor] Replace conditional ladder in requests utils #237

Open nikochiko opened 4 years ago

nikochiko commented 4 years ago

As a Google Code-In task.

Changes proposed:

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

nikochiko commented 4 years ago

@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.

krtkvrm commented 4 years ago

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?

nikochiko commented 4 years ago

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

nikochiko commented 4 years ago

@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 commented 4 years ago

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