Repository: cloudfoundry-community/cf-python-client

Issue:
Push only converts memory but not disk_quota
Hi,

I get the error "disk_quota, Error: Expected instance of Integer, given an instance of String" because the code only converts memory values:
https://github.com/cloudfoundry-community/cf-python-client/blob/3334af9bdeec36a542eaeff81adcb71a98567cd5/main/cloudfoundry_client/operations/push/validation/manifest.py#L64-L65

Best regards,
Alex

Discussion:

Patch:
diff --git a/main/cloudfoundry_client/operations/push/validation/manifest.py b/main/cloudfoundry_client/operations/push/validation/manifest.py
index 94c93dc..326bfcf 100644
--- a/main/cloudfoundry_client/operations/push/validation/manifest.py
+++ b/main/cloudfoundry_client/operations/push/validation/manifest.py
@@ -6,7 +6,7 @@ import yaml
 
 
 class ManifestReader(object):
-    MEMORY_PATTERN = re.compile(r"^(\d+)([KMGT])B?$")
+    SIZE_FIELD_PATTERNS = re.compile(r"^(\d+)([MG])B?$")
 
     POSITIVE_FIELDS = ["instances", "timeout"]
 
@@ -40,7 +40,7 @@ class ManifestReader(object):
             raise AssertionError("One of path or docker must be set")
         else:
             ManifestReader._absolute_path(manifest_directory, app_manifest)
-        ManifestReader._convert_memory(app_manifest)
+        ManifestReader._convert_size_fields(app_manifest)
         for field in ManifestReader.POSITIVE_FIELDS:
             ManifestReader._convert_positive(app_manifest, field)
         for field in ManifestReader.BOOLEAN_FIELDS:
@@ -61,25 +61,22 @@ class ManifestReader(object):
             raise AssertionError("hosts, host, domains, domain and no-hostname are all deprecated. Use the routes attribute")
 
     @staticmethod
-    def _convert_memory(manifest: dict):
-        if "memory" in manifest:
-            memory = manifest["memory"].upper()
-            match = ManifestReader.MEMORY_PATTERN.match(memory)
-            if match is None:
-                raise AssertionError("Invalid memory format: %s" % memory)
-
-            memory_number = int(match.group(1))
-            if match.group(2) == "K":
-                memory_number *= 1024
-            elif match.group(2) == "M":
-                memory_number *= 1024 * 1024
-            elif match.group(2) == "G":
-                memory_number *= 1024 * 1024 * 1024
-            elif match.group(2) == "T":
-                memory_number *= 1024 * 1024 * 1024 * 1024
-            else:
-                raise AssertionError("Invalid memory unit: %s" % memory)
-            manifest["memory"] = int(memory_number / (1024 * 1024))
+    def _convert_size_fields(manifest: dict):
+        for field_name in ["memory", "disk_quota"]:
+            if field_name in manifest:
+                field_value = manifest[field_name].upper()
+                match = ManifestReader.SIZE_FIELD_PATTERNS.match(field_value)
+                if match is None:
+                    raise AssertionError("Invalid %s format: %s" % (field_name, field_value))
+
+                size_converted = int(match.group(1))
+                if match.group(2) == "M":
+                    size_converted *= 1024 * 1024
+                elif match.group(2) == "G":
+                    size_converted *= 1024 * 1024 * 1024
+                else:
+                    raise AssertionError("Invalid %s unit: %s" % (field_name, field_value))
+                manifest[field_name] = int(size_converted / (1024 * 1024))
 
     @staticmethod
     def _convert_positive(manifest: dict, field: str):


Test patch:
diff --git a/test/operations/push/validation/test_manifest_reader.py b/test/operations/push/validation/test_manifest_reader.py
index 25995c3..0182cf2 100644
--- a/test/operations/push/validation/test_manifest_reader.py
+++ b/test/operations/push/validation/test_manifest_reader.py
@@ -104,17 +104,22 @@ class TestManifestReader(unittest.TestCase):
         ManifestReader._validate_application_manifest(".", manifest)
         self.assertEqual(os.path.abspath("test"), manifest["path"])
 
-    def test_memory_in_kb(self):
-        manifest = dict(memory="2048KB")
-        ManifestReader._convert_memory(manifest)
-        self.assertEqual(2, manifest["memory"])
-
     def test_memory_in_mb(self):
         manifest = dict(memory="2048MB")
-        ManifestReader._convert_memory(manifest)
+        ManifestReader._convert_size_fields(manifest)
         self.assertEqual(2048, manifest["memory"])
 
     def test_memory_in_gb(self):
         manifest = dict(memory="1G")
-        ManifestReader._convert_memory(manifest)
+        ManifestReader._convert_size_fields(manifest)
         self.assertEqual(1024, manifest["memory"])
+
+    def test_disk_quota_in_mb(self):
+        manifest = dict(disk_quota="2048MB")
+        ManifestReader._convert_size_fields(manifest)
+        self.assertEqual(2048, manifest["disk_quota"])
+
+    def test_disk_quota_in_gb(self):
+        manifest = dict(disk_quota="1G")
+        ManifestReader._convert_size_fields(manifest)
+        self.assertEqual(1024, manifest["disk_quota"])