Make tempurl subcommand insist on whole number seconds
Previously the tempurl subcommand would dump a traceback due to a TypeError if the seconds option was not an int value. With this patch it will now return the same error message as if the seconds option were negative or not a number. Also changes the error message to state that the seconds option should be a "whole number" rather than a "positive integer", since 0 is a valid value. Change-Id: Ie940d470f2be8006aa8eb7fe242f092457aeae21 Closes-Bug: #1621817
This commit is contained in:
		| @@ -80,17 +80,19 @@ def generate_temp_url(path, seconds, key, method, absolute=False): | |||||||
|     :param absolute: if True then the seconds parameter is interpreted as an |     :param absolute: if True then the seconds parameter is interpreted as an | ||||||
|         absolute Unix time, otherwise seconds is interpreted as a relative time |         absolute Unix time, otherwise seconds is interpreted as a relative time | ||||||
|         offset from current time. |         offset from current time. | ||||||
|     :raises: ValueError if seconds is not a positive integer or path is not to |     :raises: ValueError if seconds is not a whole number or path is not to | ||||||
|         an object. |         an object. | ||||||
|     :raises: TypeError if seconds is not an integer |  | ||||||
|     :return: the path portion of a temporary URL |     :return: the path portion of a temporary URL | ||||||
|     """ |     """ | ||||||
|     try: |     try: | ||||||
|  |         seconds = float(seconds) | ||||||
|  |         if not seconds.is_integer(): | ||||||
|  |             raise ValueError() | ||||||
|         seconds = int(seconds) |         seconds = int(seconds) | ||||||
|  |         if seconds < 0: | ||||||
|  |             raise ValueError() | ||||||
|     except ValueError: |     except ValueError: | ||||||
|         raise TypeError('seconds must be an integer') |         raise ValueError('seconds must be a whole number') | ||||||
|     if seconds < 0: |  | ||||||
|         raise ValueError('seconds must be a positive integer') |  | ||||||
|  |  | ||||||
|     if isinstance(path, six.binary_type): |     if isinstance(path, six.binary_type): | ||||||
|         try: |         try: | ||||||
|   | |||||||
| @@ -170,15 +170,30 @@ class TestTempURL(unittest.TestCase): | |||||||
|         self.assertEqual(url, expected_url) |         self.assertEqual(url, expected_url) | ||||||
|  |  | ||||||
|     def test_generate_temp_url_bad_seconds(self): |     def test_generate_temp_url_bad_seconds(self): | ||||||
|         with self.assertRaises(TypeError) as exc_manager: |         with self.assertRaises(ValueError) as exc_manager: | ||||||
|             u.generate_temp_url(self.url, 'not_an_int', self.key, self.method) |             u.generate_temp_url(self.url, 'not_an_int', self.key, self.method) | ||||||
|         self.assertEqual(exc_manager.exception.args[0], |         self.assertEqual(exc_manager.exception.args[0], | ||||||
|                          'seconds must be an integer') |                          'seconds must be a whole number') | ||||||
|  |  | ||||||
|         with self.assertRaises(ValueError) as exc_manager: |         with self.assertRaises(ValueError) as exc_manager: | ||||||
|             u.generate_temp_url(self.url, -1, self.key, self.method) |             u.generate_temp_url(self.url, -1, self.key, self.method) | ||||||
|         self.assertEqual(exc_manager.exception.args[0], |         self.assertEqual(exc_manager.exception.args[0], | ||||||
|                          'seconds must be a positive integer') |                          'seconds must be a whole number') | ||||||
|  |  | ||||||
|  |         with self.assertRaises(ValueError) as exc_manager: | ||||||
|  |             u.generate_temp_url(self.url, 1.1, self.key, self.method) | ||||||
|  |         self.assertEqual(exc_manager.exception.args[0], | ||||||
|  |                          'seconds must be a whole number') | ||||||
|  |  | ||||||
|  |         with self.assertRaises(ValueError) as exc_manager: | ||||||
|  |             u.generate_temp_url(self.url, '-1', self.key, self.method) | ||||||
|  |         self.assertEqual(exc_manager.exception.args[0], | ||||||
|  |                          'seconds must be a whole number') | ||||||
|  |  | ||||||
|  |         with self.assertRaises(ValueError) as exc_manager: | ||||||
|  |             u.generate_temp_url(self.url, '1.1', self.key, self.method) | ||||||
|  |         self.assertEqual(exc_manager.exception.args[0], | ||||||
|  |                          'seconds must be a whole number') | ||||||
|  |  | ||||||
|     def test_generate_temp_url_bad_path(self): |     def test_generate_temp_url_bad_path(self): | ||||||
|         with self.assertRaises(ValueError) as exc_manager: |         with self.assertRaises(ValueError) as exc_manager: | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user
	 Alistair Coles
					Alistair Coles