Skip to content

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

Open
hkmatsumoto wants to merge 11 commits into
Cloud-CV:masterfrom
hkmatsumoto:add-command-to-display-the-stdout-txt-data-in-terminal
Open

Add command to display the stdout.txt data in terminal#247
hkmatsumoto wants to merge 11 commits into
Cloud-CV:masterfrom
hkmatsumoto:add-command-to-display-the-stdout-txt-data-in-terminal

Conversation

@hkmatsumoto

@hkmatsumoto hkmatsumoto commented Jan 3, 2020

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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.

@Ram81 Ram81 left a comment

Copy link
Copy Markdown
Member

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

hkmatsumoto commented Jan 4, 2020

Copy link
Copy Markdown
Contributor Author

@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
Copy Markdown

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

@yashdusing

yashdusing commented Jan 6, 2020

Copy link
Copy Markdown

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

@hkmatsumoto

hkmatsumoto commented Jan 6, 2020

Copy link
Copy Markdown
Contributor Author

@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
Copy Markdown
Contributor Author

Can I get an approval on this task then?

@pushkalkatara

Copy link
Copy Markdown

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

@hkmatsumoto

hkmatsumoto commented Jan 6, 2020

Copy link
Copy Markdown
Contributor Author

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

@krtkvrm krtkvrm left a comment

Copy link
Copy Markdown
Member

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
Copy Markdown
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?

@hkmatsumoto hkmatsumoto requested a review from krtkvrm January 7, 2020 05:10
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.

5 participants