Add command to setup the CLI

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

Add command to setup the CLI #217

Open nikochiko opened 4 years ago

nikochiko commented 4 years ago

Changes proposed in this PR:

Update login.py to allow calling from another command
Added command to set up the CLI configuration like host and auth token

Command example:

evalai setup -u USER -p PASSWORD [-h HOST]

Added unit tests for the above command still in progress, but thought I could gain some feedback on the main code in the meanwhile:

P.S.: I tried using a fancy name and showing EvalAI in Text Art form to make it a little interesting 😅 . Please let me know if that needs to be changed.

Mentors: @vkartik97 @pushkalkatara @Ram81 @RishabhJain2018 @yashdusing

nikochiko commented 4 years ago

Also want to point this out: evalai/login.py#L20-L33 :

This block can be replaced with the set_token command to maintain consistency and following DRY.

Also, there is a possible bug in the code: the except block doesn't end with sys.exit(1) . Hence, the final "Logged in successfully!" will be printed even if the code threw an OSError/IOError.

nikochiko commented 4 years ago

Ah, pardon me for the mess of commits here. I should've squashed and merged 😅 .

Anyways, a lot of changes proposed in this PR :smile::

Refactor code so that multiple commands can use the same piece of code.
Removed the previous dependency on other commands, instead both use same helper functions to maintain similar error logs.
Added exit codes at a few place.
Added unit tests for everything new (setup command, refactored auth utils)

This is in addition to the above description ☝️

Thanks!

nikochiko commented 4 years ago

@yashdusing the new command is completely covered (coveralls report). The reason for the coverage going down is the part I refactored for usage in multiple places. I am working on it right now and will update the PR with the tests in a few hours!

nikochiko commented 4 years ago

@yashdusing Fixed both issues! 😄

yashdusing commented 4 years ago

LGTM 👍 @Ram81 @pushkalkatara @vkartik97 @RishabhJain2018 please review

yashdusing commented 4 years ago

@nikochiko could you please attach a link to a gif to the PR demonstrating the new command ?

nikochiko commented 4 years ago

@yashdusing Made the gif: Successful with prompt: setup-demo

When login fails, revert message: evalai-demo-2

nikochiko commented 4 years ago

@vkartik97 Done! 😄 The new line slipped in during the merging 😸 Fixed it now!