You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@submarine.apache.org by GitBox <gi...@apache.org> on 2020/06/23 00:39:20 UTC
[GitHub] [submarine] pingsutw opened a new pull request #323: SUBMARINE-542. [SDK] get_log error when experiment is not started
pingsutw opened a new pull request #323:
URL: https://github.com/apache/submarine/pull/323
### What is this PR for?
Fix get_log error when the experiment is not started
```bash
IndexError Traceback (most recent call last)
<ipython-input-13-8bf55eff97d1> in <module>
----> 1 submarine_client.get_log(id)~/opt/anaconda3/envs/python3-7/lib/python3.7/site-packages/submarine/experiment/api/experiment_client.py in get_log(self, id, master)
124
125 if master is True:
--> 126 log_contents = [log_contents[0]]
127
128 for log_content in log_contents:IndexError: list index out of range
```
### What type of PR is it?
[Bug Fix]
### Todos
* [ ] - Task
### What is the Jira issue?
https://issues.apache.org/jira/browse/SUBMARINE-542
### How should this be tested?
https://travis-ci.org/github/pingsutw/hadoop-submarine/builds/701089803
### Screenshots (if appropriate)
### Questions:
* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] tangzhankun commented on a change in pull request #323: SUBMARINE-542. [SDK] get_log error when experiment is not started
Posted by GitBox <gi...@apache.org>.
tangzhankun commented on a change in pull request #323:
URL: https://github.com/apache/submarine/pull/323#discussion_r444141005
##########
File path: submarine-sdk/pysubmarine/submarine/experiment/api/experiment_client.py
##########
@@ -110,19 +113,19 @@ def delete_experiment(self, id):
response = self.experiment_api.delete_experiment(id)
return response.result
- def get_log(self, id, master=True):
+ def get_log(self, id, master=False):
"""
Get training logs of the experiment.
By default only get the logs of Pod that has labels 'job-role: master'.
- :param master: By default get pod with label 'job-role: master' pod if True.
+ :param master: Get pod with label 'job-role: master' pod if True.
Review comment:
How about this?
```Python
"""
get_logs(self, id, onlyMaster=False)
Get training logs of all pod of the experiment.
By default get all the logs of Pod
:param id: experiment id
:param onlyMaster: By default include pod log of "master" which might be Tensorflow PS/Chief or PyTroch master
:return: str: pods logs
"""
```
get_logs(id) will get all pod logs
get_long(id, onlyMaster=True) will only get master pod's log
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] tangzhankun commented on a change in pull request #323: SUBMARINE-542. [SDK] get_log error when experiment is not started
Posted by GitBox <gi...@apache.org>.
tangzhankun commented on a change in pull request #323:
URL: https://github.com/apache/submarine/pull/323#discussion_r443928052
##########
File path: submarine-sdk/pysubmarine/submarine/experiment/api/experiment_client.py
##########
@@ -110,19 +113,19 @@ def delete_experiment(self, id):
response = self.experiment_api.delete_experiment(id)
return response.result
- def get_log(self, id, master=True):
+ def get_log(self, id, master=False):
"""
Get training logs of the experiment.
By default only get the logs of Pod that has labels 'job-role: master'.
- :param master: By default get pod with label 'job-role: master' pod if True.
+ :param master: Get pod with label 'job-role: master' pod if True.
Review comment:
The description of "master" param seems confusing. I thought get_log(id) will get all pod's log, and get_log(id, master=false) will discard the master pod log.
But per the description, get_log(id, master=True) will get a master log. This is confusing.
> Get pod with label 'job-role: master' pod if True.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] tangzhankun commented on a change in pull request #323: SUBMARINE-542. [SDK] get_log error when experiment is not started
Posted by GitBox <gi...@apache.org>.
tangzhankun commented on a change in pull request #323:
URL: https://github.com/apache/submarine/pull/323#discussion_r444141005
##########
File path: submarine-sdk/pysubmarine/submarine/experiment/api/experiment_client.py
##########
@@ -110,19 +113,19 @@ def delete_experiment(self, id):
response = self.experiment_api.delete_experiment(id)
return response.result
- def get_log(self, id, master=True):
+ def get_log(self, id, master=False):
"""
Get training logs of the experiment.
By default only get the logs of Pod that has labels 'job-role: master'.
- :param master: By default get pod with label 'job-role: master' pod if True.
+ :param master: Get pod with label 'job-role: master' pod if True.
Review comment:
How about this?
```Python
"""
get_logs(self, id, onlyMaster=False)
Get training logs of all pod of the experiment.
By default get all the logs of Pod
:param id: experiment id
:param onlyMaster: By default include pod log of "master" which might be Tensorflow PS/Chief or PyTroch master
:return: str: pods logs
"""
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] tangzhankun commented on a change in pull request #323: SUBMARINE-542. [SDK] get_log error when experiment is not started
Posted by GitBox <gi...@apache.org>.
tangzhankun commented on a change in pull request #323:
URL: https://github.com/apache/submarine/pull/323#discussion_r444141005
##########
File path: submarine-sdk/pysubmarine/submarine/experiment/api/experiment_client.py
##########
@@ -110,19 +113,19 @@ def delete_experiment(self, id):
response = self.experiment_api.delete_experiment(id)
return response.result
- def get_log(self, id, master=True):
+ def get_log(self, id, master=False):
"""
Get training logs of the experiment.
By default only get the logs of Pod that has labels 'job-role: master'.
- :param master: By default get pod with label 'job-role: master' pod if True.
+ :param master: Get pod with label 'job-role: master' pod if True.
Review comment:
How about this?
```Python
"""
get_logs(self, id, onlyMaster=False)
Get training logs of all pod of the experiment.
By default get all the logs of Pod
:param id: experiment id
:param onlyMaster: By default include pod log of "master" which might be Tensorflow PS/Chief or PyTroch master
:return: str: pods logs
"""
```
get_logs(id) will get all pod logs
get_long(id, onlyMaster=True) will only get master pod's log
@pingsutw
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] pingsutw commented on a change in pull request #323: SUBMARINE-542. [SDK] get_log error when experiment is not started
Posted by GitBox <gi...@apache.org>.
pingsutw commented on a change in pull request #323:
URL: https://github.com/apache/submarine/pull/323#discussion_r444662054
##########
File path: submarine-sdk/pysubmarine/submarine/experiment/api/experiment_client.py
##########
@@ -110,19 +113,19 @@ def delete_experiment(self, id):
response = self.experiment_api.delete_experiment(id)
return response.result
- def get_log(self, id, master=True):
+ def get_log(self, id, master=False):
"""
Get training logs of the experiment.
By default only get the logs of Pod that has labels 'job-role: master'.
- :param master: By default get pod with label 'job-role: master' pod if True.
+ :param master: Get pod with label 'job-role: master' pod if True.
Review comment:
Thanks, @tangzhankun.
it's more clear than before. I've updated it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] pingsutw commented on a change in pull request #323: SUBMARINE-542. [SDK] get_log error when experiment is not started
Posted by GitBox <gi...@apache.org>.
pingsutw commented on a change in pull request #323:
URL: https://github.com/apache/submarine/pull/323#discussion_r444110040
##########
File path: submarine-sdk/pysubmarine/submarine/experiment/api/experiment_client.py
##########
@@ -110,19 +113,19 @@ def delete_experiment(self, id):
response = self.experiment_api.delete_experiment(id)
return response.result
- def get_log(self, id, master=True):
+ def get_log(self, id, master=False):
"""
Get training logs of the experiment.
By default only get the logs of Pod that has labels 'job-role: master'.
- :param master: By default get pod with label 'job-role: master' pod if True.
+ :param master: Get pod with label 'job-role: master' pod if True.
Review comment:
I just follow the convention in `tf-operator`
https://github.com/kubeflow/tf-operator/blob/cf6ebd6d2d9f679646dad3530e9314f63505f98a/sdk/python/kubeflow/tfjob/api/tf_job_client.py#L356-L371
Maybe we could change the description, so it will be more clear for the users.
Any suggestion for the comments and param in `get_log`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] asfgit closed pull request #323: SUBMARINE-542. [SDK] get_log error when experiment is not started
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #323:
URL: https://github.com/apache/submarine/pull/323
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org