Improve get_experiment and list_runs in the python sdk #508
Conversation
gaoning777
changed the title
|
/lgtm |
|
/test kubeflow-pipeline-sample-test |
| raise ValueError('Either experiment_id or experiment_name is required') | ||
| if experiment_id is not None: | ||
| return self._experiment_api.get_experiment(id=experiment_id) | ||
| next_page_token = '' |
There was a problem hiding this comment.
What do you think about having a separate get_experiment_by_name function? Calling it would be a bit shorter and the function code will be simpler?
get_experiment_by_name('foo')
vs
get_experiment(experiment_name='foo')
There was a problem hiding this comment.
Bradley and I discussed this yesterday. It seems more natural and simple to have get_experiment that takes either name or id. Or else, it would rather be two functions named get_experiment_by_id and get_experiment_by_name, which would break the backward compatibility because we still want to support the get_experiment function.
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gaoning777 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
1 similar comment
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gaoning777 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
* add get_experiment_id and list_runs_by_experiment * offer only one get_experiment function * return experiment body instead of id * simply codes * simply code 2 * remove experiment_id check in the while loop * minor bug
resolving #506
/cc qimingj Ark-kun
This change is