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

Add command to display the stdout.txt data in terminal #247

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

Add command to display the stdout.txt data in terminal #247

wants to merge 11 commits into from

Conversation

hkmatsumoto
Copy link
Contributor

@hkmatsumoto hkmatsumoto commented Jan 3, 2020

Link to the GCI Task:

Changes:

  • Add a subcommand to show the content of stdout file of the specified submission, invoked by evalai submission SUBMISSION_ID stdout.
  • Add a helper function of the subcommand.

CC: @yashdusing @RishabhJain2018 @pushkalkatara @Ram81 @vkartik97

@hkmatsumoto
Copy link
Contributor Author

Being different from display_submission_result, I didn't add error handling codes for requests.exceptions.MissingSchema because they will not be called even if I add them.
See #248 for detail.

Copy link
Member

@Ram81 Ram81 left a comment

Choose a reason for hiding this comment

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

@takitsuse can you share a screenshot of command output

@hkmatsumoto
Copy link
Contributor Author

hkmatsumoto commented Jan 4, 2020

@Ram81
The content of stdout file:
Screenshot from 2020-01-05 03-17-37

The command output:
Screenshot from 2020-01-05 03-17-23

@hkmatsumoto hkmatsumoto requested a review from Ram81 January 4, 2020 18:21
@pushkalkatara
Copy link

Looks good to me. @Ram81 let's approve this?

@yashdusing
Copy link

yashdusing commented Jan 6, 2020

@takitsuse Should we add a failure test case in this PR as well ?
If not, then the PR LGTM

@hkmatsumoto
Copy link
Contributor Author

hkmatsumoto commented Jan 6, 2020

@yashdusing I don't think we should add it as long as #248 isn't resolved and reachable error handling can't be added.

@hkmatsumoto
Copy link
Contributor Author

Can I get an approval on this task then?

@pushkalkatara
Copy link

Let's approve this @yashdusing We can create a new GCI task with it if required.
Approved @takitsuse

@hkmatsumoto
Copy link
Contributor Author

hkmatsumoto commented Jan 6, 2020

@Ram81 @pushkalkatara @yashdusing
It's embarrassing to say this but, it seems I misread the codes due to lack of experience about try-except clause. The error handling code was reachable in the first place.
I managed to add error handling code that is tested, closing #248.
Sorry for the confusion.

Copy link
Member

@krtkvrm krtkvrm left a comment

Choose a reason for hiding this comment

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

@takitsuse
Can you also add the command to docs: evalai-cli/docs/index.html

@hkmatsumoto
Copy link
Contributor Author

@vkartik97 Added 👍
By the way, I think I can implement a similar command for stderr file. Could you make a new GCI task for it if it is okay for you?

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.

None yet

5 participants