Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #119: Add command & feature for Creating challenge. #149

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

KhalidRmb
Copy link
Member

@KhalidRmb KhalidRmb commented Mar 8, 2019

Fixes #119
Adds a subcommand to the challenges command to create the challenge using the Host team ID and a Zip file.
Usage: evalai challenges create --file FILE TEAM where FILE is the zip file and TEAM is the Host team ID.

  • Add command.
  • Add tests.

Screenshot from 2019-03-09 21-21-06

@KhalidRmb KhalidRmb changed the title Fix #119: Add command & feature for Creating challenge. [WIP] Fix #119: Add command & feature for Creating challenge. Mar 8, 2019
@KhalidRmb KhalidRmb changed the title [WIP] Fix #119: Add command & feature for Creating challenge. Fix #119: Add command & feature for Creating challenge. Mar 11, 2019
@KhalidRmb
Copy link
Member Author

KhalidRmb commented Mar 11, 2019

@RishabhJain2018 @deshraj Please review. Thanks! @isht3

@KhalidRmb KhalidRmb changed the title Fix #119: Add command & feature for Creating challenge. [REVIEW PENDING] Fix #119: Add command & feature for Creating challenge. Mar 15, 2019
@KhalidRmb KhalidRmb requested a review from RishabhJain2018 May 10, 2019 12:10
@@ -1,6 +1,8 @@
import json
import responses

import os
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@KhalidRmb can you arrange the imports in lexicographic order

@@ -1,6 +1,8 @@
import json
import responses

import os
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here


@responses.activate
def test_create_challenge_when_id_is_not_valid(self):
expected = ("Could not establish a connection to EvalAI."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@KhalidRmb not sure if this is the correct message for invalid challenge id, user should know the mistake is in challenge id. @RishabhJain2018 your input?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think he is referring to host team id. Let him confirm first.

Copy link
Member Author

@KhalidRmb KhalidRmb May 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am referring to the host team ID here. For a host id invalid message to be displayed, the API must send a message back saying why creation failed. Based on that we must modify the cli code. @RishabhJain2018 @Ram81

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I was referring to host team id

echo(err)
sys.exit(1)
except requests.exceptions.RequestException as err:
echo(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@KhalidRmb can we change this to a message like invalid host team id if the issue is with team id otherwise the request error message

Copy link
Member Author

@KhalidRmb KhalidRmb May 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this will require modification in the API, since there's no code to send an apt response for invalid host id here: https://github.com/Cloud-CV/EvalAI/blob/d5f28ae941dae1e50e623a7b880fefbacea3ef4b/apps/challenges/views.py#L606
Based on it, we can modify the cli code.

@KhalidRmb KhalidRmb changed the title [REVIEW PENDING] Fix #119: Add command & feature for Creating challenge. Fix #119: Add command & feature for Creating challenge. May 13, 2019
@KhalidRmb
Copy link
Member Author

@Ram81 PTAL.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add command for creating a challenge
3 participants