[batch] Refactor resource billing checks with additional debugging in…

…fo (#12713) * [batch] Refactor resource billing checks with additional debugging info * delint
hail-is · Feb 18, 2023 · 537f8f7 · 537f8f7
1 parent 2d65cf5
commit 537f8f7
Showing 1 changed file with 38 additions and 36 deletions.
diff --git a/batch/batch/driver/billing_manager.py b/batch/batch/driver/billing_manager.py
@@ -62,50 +62,52 @@ async def _refresh_resources_from_retail_prices(self, prices: List[Price]):
 
             current_product_version = self.product_versions.latest_version(product)
 
-            if current_product_version is None:
+            is_new_product = current_product_version is None
+
+            if is_new_product:
                 resource_name = product_version_to_resource(product, latest_product_version)
                 resource_updates.append((resource_name, latest_resource_rate))
                 product_version_updates.append((product, latest_product_version))
                 log.info(
                     f'adding new resource {resource_name} {latest_product_version} with rate change of {latest_resource_rate}'
                 )
-                continue
-
-            resource_name = product_version_to_resource(product, current_product_version)
-            current_resource_rate = self.resource_rates.get(resource_name)
-
-            if current_product_version == latest_product_version and current_resource_rate != latest_resource_rate:
-                log.error(
-                    f'resource {resource_name} does not have the latest rate in the database for '
-                    f'version {current_product_version}: {current_resource_rate} vs {latest_resource_rate}; '
-                    f'did the vm price change without a version change?'
-                )
-                continue
-
-            if current_product_version != latest_product_version and current_resource_rate == latest_resource_rate:
-                # this prevents having too many resources in the database with redundant information
-                log.info(
-                    f'ignoring price update for product {product} -- the latest rate is equal to the previous rate'
-                )
-                continue
-
-            if current_product_version != latest_product_version and current_resource_rate != latest_resource_rate:
-                if price.is_current_price():
-                    resource_updates.append((resource_name, latest_resource_rate))
-                    product_version_updates.append((product, latest_product_version))
+            else:
+                assert current_product_version
+                resource_name = product_version_to_resource(product, current_product_version)
+                current_resource_rate = self.resource_rates.get(resource_name)
+
+                have_latest_version = current_product_version == latest_product_version
+                have_latest_rate = current_resource_rate == latest_resource_rate
+
+                if have_latest_version and not have_latest_rate:
+                    log.error(
+                        f'resource {resource_name} does not have the latest rate in the database for '
+                        f'version {current_product_version}: {current_resource_rate} vs {latest_resource_rate}; '
+                        f'did the vm price change without a version change?'
+                    )
+                elif not have_latest_version and have_latest_rate:
+                    # this prevents having too many resources in the database with redundant information
                     log.info(
-                        f'resource {resource_name} changed from {current_product_version} to {latest_product_version} with rate change of '
-                        f'({current_resource_rate}) => ({latest_resource_rate})'
+                        f'ignoring price update for product {product} -- the latest rate is equal to the previous rate '
+                        f'({current_product_version}) => ({latest_product_version}) with rate {latest_resource_rate}'
                     )
-                    continue
-                log.error(
-                    f'price changed but the price is not current {product} ({current_product_version}) => ({latest_product_version}) ({current_resource_rate}) => ({latest_resource_rate}) '
-                    f'{price.effective_start_date} {price.effective_end_date}'
-                )
-
-            assert (
-                current_product_version == latest_product_version and current_resource_rate == latest_resource_rate
-            ), f'{current_product_version} {latest_product_version} {current_resource_rate} {latest_resource_rate}'
+                elif not have_latest_version and not have_latest_rate:
+                    if price.is_current_price():
+                        resource_updates.append((resource_name, latest_resource_rate))
+                        product_version_updates.append((product, latest_product_version))
+                        log.info(
+                            f'resource {resource_name} changed from {current_product_version} to {latest_product_version} with rate change of '
+                            f'({current_resource_rate}) => ({latest_resource_rate})'
+                        )
+                    else:
+                        log.error(
+                            f'price changed but the price is not current {product} ({current_product_version}) => ({latest_product_version}) ({current_resource_rate}) => ({latest_resource_rate}) '
+                            f'{price.effective_start_date} {price.effective_end_date}'
+                        )
+                else:
+                    assert (
+                        have_latest_version and have_latest_rate
+                    ), f'{current_product_version} {latest_product_version} {current_resource_rate} {latest_resource_rate}'
 
         @transaction(self.db)
         async def insert_or_update(tx):