Fix tuning job name generation (aws#35)

Author: laurenyu

src/sagemaker/tuner.py

@@ -189,6 +189,8 @@ def _validate_parameter_ranges(self):
 
 
 class _TuningJob(_Job):
+    TUNING_JOB_NAME_MAX_LENGTH = 32
+
     def __init__(self, sagemaker_session, tuning_job_name):
         super(_TuningJob, self).__init__(sagemaker_session, tuning_job_name)
 
@@ -205,12 +207,8 @@ def start_new(cls, tuner, inputs):
         """
         config = _Job._load_config(inputs, tuner.estimator)
 
-        base_name = tuner.estimator.base_job_name or base_name_from_image(tuner.estimator.train_image())
-        tuning_job_name = name_from_base(base_name)
-
-        # TODO: Update name generation so that the base name isn't limited to so few characters
-        if len(tuning_job_name) > 32:
-            raise ValueError('Tuning job name too long - must be 32 characters or fewer: {}'.format(tuning_job_name))
+        base_name = tuner.base_tuning_job_name or base_name_from_image(tuner.estimator.train_image())
+        tuning_job_name = name_from_base(base_name, max_length=cls.TUNING_JOB_NAME_MAX_LENGTH, short=True)
 
         tuner.estimator.sagemaker_session.tune(job_name=tuning_job_name, strategy=tuner.strategy,
                                                objective_type=tuner.objective_type,

src/sagemaker/utils.py

@@ -31,20 +31,21 @@ def name_from_image(image):
     return name_from_base(base_name_from_image(image))
 
 
-def name_from_base(base):
+def name_from_base(base, max_length=63, short=False):
     """Append a timestamp to the provided string.
 
-    The appended timestamp is precise to the millisecond. This function assures that the total length of the resulting
-    string is not longer that 63, trimming the input parameter if necessary.
+    This function assures that the total length of the resulting string is not
+    longer than the specified max length, trimming the input parameter if necessary.
 
     Args:
         base (str): String used as prefix to generate the unique name.
+        max_length (int): Maximum length for the resulting string.
+        short (bool): Whether or not to use a truncated timestamp.
 
     Returns:
-        str: Input parameter with appended timestamp (no longer than 63 characters).
+        str: Input parameter with appended timestamp.
     """
-    max_length = 63
-    timestamp = sagemaker_timestamp()
+    timestamp = sagemaker_short_timestamp() if short else sagemaker_timestamp()
     trimmed_base = base[:max_length - len(timestamp) - 1]
     return '{}-{}'.format(trimmed_base, timestamp)
 
@@ -70,6 +71,11 @@ def sagemaker_timestamp():
     return time.strftime("%Y-%m-%d-%H-%M-%S-{}".format(moment_ms), time.gmtime(moment))
 
 
+def sagemaker_short_timestamp():
+    """Return a timestamp that is relatively short in length"""
+    return time.strftime('%y%m%d-%H%M')
+
+
 def debug(func):
     """Print the function name and arguments for debugging."""
     @wraps(func)

tests/unit/test_utils.py

@@ -12,7 +12,11 @@
 # language governing permissions and limitations under the License.
 from __future__ import absolute_import
 
-from sagemaker.utils import get_config_value
+from mock import patch
+
+from sagemaker.utils import get_config_value, name_from_base
+
+NAME = 'base_name'
 
 
 def test_get_config_value():
@@ -32,3 +36,15 @@ def test_get_config_value():
 
     assert get_config_value('does_not.exist', config) is None
     assert get_config_value('other.key', None) is None
+
+
+@patch('sagemaker.utils.sagemaker_timestamp')
+def test_name_from_base(sagemaker_timestamp):
+    name_from_base(NAME, short=False)
+    assert sagemaker_timestamp.called_once
+
+
+@patch('sagemaker.utils.sagemaker_short_timestamp')
+def test_name_from_base_short(sagemaker_short_timestamp):
+    name_from_base(NAME, short=True)
+    assert sagemaker_short_timestamp.called_once