Add search for training job and fix slow tests #71
Conversation
| creation_time = None | ||
| created_by = None | ||
| last_modified_time = None | ||
| last_modified_by = None |
There was a problem hiding this comment.
these are in the response see DescribeExperiment.xml
There was a problem hiding this comment.
yeah, I removed those since those will be covered by **kwargs
| training_job_name = None | ||
| training_job_arn = None | ||
| tuning_job_arn = None | ||
| labeling_job_arn = None | ||
| autoML_job_arn = None | ||
| model_artifacts = None | ||
| training_job_status = None | ||
| hyper_parameters = None | ||
| algorithm_specification = None | ||
| input_data_config = None | ||
| output_data_config = None | ||
| resource_config = None | ||
| debug_hook_config = None | ||
| experiment_config = None | ||
| debug_rule_config = None |
There was a problem hiding this comment.
how did you construct this list of fields? what about some other fields like SecondaryStatus and FailureReason?
There was a problem hiding this comment.
I added the ones I think are most useful, but others should be covered by **kwargs
There was a problem hiding this comment.
seems incomplete tho, we will get back all the fields in search regardless of what is in kwargs. i guess we can follow up later and add the remaining fields.
| @@ -20,12 +20,12 @@ | |||
|
|
|||
| @pytest.mark.slow | |||
| def test_track_from_processing_job(sagemaker_boto_client, processing_job_name): | |||
|
|
|||
| processing_job_name = "smexperiments-integ-549de818-b4bd-42d0-8ce7-1cb4cb3573d9" | |||
There was a problem hiding this comment.
meant to have this hard coded job name here?
There was a problem hiding this comment.
good point, I removed this and also training_job_name which I dont think is needed.
| @@ -1,4 +1,4 @@ | |||
| # Copyright 019 Amazon.com, Inc. or its affiliates. All Rights Reserved. | |||
| # Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved. | |||
There was a problem hiding this comment.
this should be done automatically, i created an issue
tests/integ/test_training_job.py
Outdated
| for s in TrainingJob.search( | ||
| search_expression=search_expression, max_results=10, sagemaker_boto_client=sagemaker_boto_client | ||
| ): | ||
| training_job_names_searched.append(s.training_job_name) |
There was a problem hiding this comment.
sure, just black-formatted it
tests/integ/test_training_job.py
Outdated
| training_job_names_searched.append(s.training_job_name) | ||
|
|
||
| assert len(training_job_names_searched) == 1 | ||
| assert training_job_names_searched # sanity test |
There was a problem hiding this comment.
can we have a stronger assertion here?
I agree, sorry, just started fixing slow test since its blocking my search integ tests. |
Issue #, if available:
Description of changes:
Testing done:
Merge Checklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your pull request.General
Tests
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.