evalai-cli
evalai-cli copied to clipboard
Add command to setup the CLI
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
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
.
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!
@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!
@yashdusing Fixed both issues! 😄
LGTM 👍 @Ram81 @pushkalkatara @vkartik97 @RishabhJain2018 please review
@nikochiko could you please attach a link to a gif to the PR demonstrating the new command ?
@yashdusing Made the gif:
Successful with prompt:
When login fails, revert message:
@vkartik97 Done! 😄 The new line slipped in during the merging 😸 Fixed it now!