Skip to content

Commit 6d9278a

Browse files
committed
azure: simplify the submission targets usage
This commit simplifies the methods from `AzureService` to require a single `target` whenever retrieving a `product` or `plan`, instead of automatically trying to find it from all targets. With that the `publish` function will always have to pass the correct targets and the code gets way more predictable and simplified. It also helps to reduce the amount of requests for the Microsoft Graph API as the `get_*` will always perform a single request to retrieve the data in a particular target. However, there's a tiny flaw: It will always raise `NotFoundError` whenever an unexisintg target is passed. Not all products have all 3 targets in their submission states, so this issue will be addresed in the next commit. Refers to SPSTRAT-595 Signed-off-by: Jonathan Gangi <[email protected]>
1 parent 0bd8ffe commit 6d9278a

File tree

2 files changed

+89
-141
lines changed

2 files changed

+89
-141
lines changed

cloudpub/ms_azure/service.py

Lines changed: 51 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,7 @@ def list_products(self) -> List[ProductSummary]:
244244
self._products = [p for p in self.products]
245245
return self._products
246246

247-
def get_product(self, product_id: str, first_target: str = "preview") -> Product:
247+
def get_product(self, product_id: str, target: str) -> Product:
248248
"""
249249
Return the requested Product by its ID.
250250
@@ -256,34 +256,31 @@ def get_product(self, product_id: str, first_target: str = "preview") -> Product
256256
Args:
257257
product_durable_id (str)
258258
The product UUID
259-
first_target (str, optional)
260-
The first target to lookup into. Defaults to ``preview``.
259+
target (str)
260+
The submission target to retrieve the product from.
261261
Returns:
262262
Product: the requested product
263263
"""
264-
targets = list_all_targets(start_with=first_target)
265-
266-
for t in targets:
267-
log.info("Requesting the product ID \"%s\" with state \"%s\".", product_id, t)
268-
try:
269-
resp = self.session.get(
270-
path=f"/resource-tree/product/{product_id}", params={"targetType": t}
271-
)
272-
data = self._assert_dict(resp)
273-
return Product.from_json(data)
274-
except (ValueError, HTTPError):
275-
log.debug("Couldn't find the product \"%s\" with state \"%s\"", product_id, t)
264+
log.info("Requesting the product ID \"%s\" with state \"%s\".", product_id, target)
265+
try:
266+
resp = self.session.get(
267+
path=f"/resource-tree/product/{product_id}", params={"targetType": target}
268+
)
269+
data = self._assert_dict(resp)
270+
return Product.from_json(data)
271+
except (ValueError, HTTPError):
272+
log.debug("Couldn't find the product \"%s\" with state \"%s\"", product_id, target)
276273
self._raise_error(NotFoundError, f"No such product with id \"{product_id}\"")
277274

278-
def get_product_by_name(self, product_name: str, first_target: str = "preview") -> Product:
275+
def get_product_by_name(self, product_name: str, target: str) -> Product:
279276
"""
280277
Return the requested Product by its name from Legacy CPP API.
281278
282279
Args:
283280
product_name (str)
284281
The product name according to Legacy CPP API.
285-
first_target (str, optional)
286-
The first target to lookup into. Defaults to ``preview``.
282+
target (str, optional)
283+
The submission target to retrieve the product from.
287284
Returns:
288285
Product: the requested product when found
289286
Raises:
@@ -292,7 +289,7 @@ def get_product_by_name(self, product_name: str, first_target: str = "preview")
292289
for product in self.products:
293290
if product.identity.name == product_name:
294291
log.debug("Product alias \"%s\" has the ID \"%s\"", product_name, product.id)
295-
return self.get_product(product.id, first_target=first_target)
292+
return self.get_product(product.id, target=target)
296293
self._raise_error(NotFoundError, f"No such product with name \"{product_name}\"")
297294

298295
def get_submissions(self, product_id: str) -> List[ProductSubmission]:
@@ -380,45 +377,41 @@ def get_product_plan_by_name(
380377
self,
381378
product_name: str,
382379
plan_name: str,
383-
first_target: str = "preview",
384-
) -> Tuple[Product, PlanSummary, str]:
380+
target: str,
381+
) -> Tuple[Product, PlanSummary]:
385382
"""Return a tuple with the desired Product and Plan after iterating over all targets.
386383
387384
Args:
388385
product_name (str): The name of the product to search for
389386
plan_name (str): The name of the plan to search for
390-
first_target (str, optional)
391-
The first target to lookup into. Defaults to ``preview``.
387+
target (str)
388+
The submission target to retrieve the product/plan from.
392389
Returns:
393-
Tuple[Product, PlanSummary, str]: The Product, PlanSummary and target when fonud
390+
Tuple[Product, PlanSummary]: The Product and PlanSummary when fonud
394391
Raises:
395-
NotFoundError whenever all targets are exhausted and no information was found
396-
"""
397-
targets = list_all_targets(start_with=first_target)
398-
399-
for tgt in targets:
400-
try:
401-
product = self.get_product_by_name(product_name, first_target=tgt)
402-
plan = self.get_plan_by_name(product, plan_name)
403-
return product, plan, tgt
404-
except NotFoundError:
405-
continue
406-
self._raise_error(
407-
NotFoundError, f"No such plan with name \"{plan_name} for {product_name}\""
408-
)
392+
NotFoundError whenever no information was found in the respective submission target.
393+
"""
394+
try:
395+
product = self.get_product_by_name(product_name, target=target)
396+
plan = self.get_plan_by_name(product, plan_name)
397+
return product, plan
398+
except NotFoundError:
399+
self._raise_error(
400+
NotFoundError, f"No such plan with name \"{plan_name} for {product_name}\""
401+
)
409402

410-
def diff_offer(self, product: Product, first_target="preview") -> DeepDiff:
403+
def diff_offer(self, product: Product, target: str) -> DeepDiff:
411404
"""Compute the difference between the provided product and the one in the remote.
412405
413406
Args:
414407
product (Product)
415408
The local product to diff with the remote one.
416-
first_target (str)
417-
The first target to lookup into. Defaults to ``preview``.
409+
target (str)
410+
The submission target to retrieve the product from.
418411
Returns:
419412
DeepDiff: The diff data.
420413
"""
421-
remote = self.get_product(product.id, first_target=first_target)
414+
remote = self.get_product(product.id, target=target)
422415
return DeepDiff(remote.to_json(), product.to_json(), exclude_regex_paths=self.DIFF_EXCLUDES)
423416

424417
def submit_to_status(
@@ -661,6 +654,7 @@ def _overwrite_disk_version(
661654
product_name: str,
662655
plan_name: str,
663656
source: VMImageSource,
657+
target: str,
664658
) -> TechnicalConfigLookUpData:
665659
"""Private method to overwrite the technical config with a new DiskVersion.
666660
@@ -669,15 +663,16 @@ def _overwrite_disk_version(
669663
product_name (str): the product (offer) name
670664
plan_name (str): the plan name
671665
source (VMImageSource): the source VMI to create and overwrite the new DiskVersion
666+
target (str): the submission target.
672667
673668
Returns:
674669
TechnicalConfigLookUpData: The overwritten tech_config for the product/plan
675670
"""
676-
product, plan, tgt = self.get_product_plan_by_name(product_name, plan_name)
671+
product, plan = self.get_product_plan_by_name(product_name, plan_name, target)
677672
log.warning(
678673
"Overwriting the plan \"%s\" on \"%s\" with the given image: \"%s\".",
679674
plan_name,
680-
tgt,
675+
target,
681676
metadata.image_path,
682677
)
683678
tech_config = self.get_plan_tech_config(product, plan)
@@ -689,7 +684,7 @@ def _overwrite_disk_version(
689684
"sas_found": False,
690685
"product": product,
691686
"plan": plan,
692-
"target": tgt,
687+
"target": target,
693688
}
694689

695690
def _look_up_sas_on_technical_config(
@@ -706,13 +701,11 @@ def _look_up_sas_on_technical_config(
706701
Returns:
707702
TechnicalConfigLookUpData: The data retrieved for the given submission target.
708703
"""
709-
product, plan, tgt = self.get_product_plan_by_name(
710-
product_name, plan_name, first_target=target
711-
)
704+
product, plan = self.get_product_plan_by_name(product_name, plan_name, target)
712705
log.info(
713706
"Retrieving the technical config for \"%s\" on \"%s\".",
714707
metadata.destination,
715-
tgt,
708+
target,
716709
)
717710
tech_config = self.get_plan_tech_config(product, plan)
718711
sas_found = False
@@ -721,7 +714,7 @@ def _look_up_sas_on_technical_config(
721714
log.info(
722715
"The destination \"%s\" on \"%s\" already contains the SAS URI: \"%s\".",
723716
metadata.destination,
724-
tgt,
717+
target,
725718
metadata.image_path,
726719
)
727720
sas_found = True
@@ -731,7 +724,7 @@ def _look_up_sas_on_technical_config(
731724
"sas_found": sas_found,
732725
"product": product,
733726
"plan": plan,
734-
"target": tgt,
727+
"target": target,
735728
}
736729

737730
def _create_or_update_disk_version(
@@ -803,19 +796,18 @@ def publish(self, metadata: AzurePublishingMetadata) -> None:
803796
# Note: If `overwrite` is True it means we can set this VM image as the only one in the
804797
# plan's technical config and discard all other VM images which may've been present.
805798
if metadata.overwrite is True:
806-
res = self._overwrite_disk_version(metadata, product_name, plan_name, source)
807-
tgt = res["target"]
799+
target = "draft"
800+
res = self._overwrite_disk_version(metadata, product_name, plan_name, source, target)
808801
tech_config = res["tech_config"]
809802
disk_version = tech_config.disk_versions[0] # only 1 as it was overwritten
810803
else:
811804
# Otherwise we need to check whether SAS isn't already present
812805
# in any of the targets "preview", "live" or "draft" and if not attach and publish it.
813-
for target in list_all_targets(start_with="preview"):
806+
for target in list_all_targets():
814807
res = self._look_up_sas_on_technical_config(
815808
metadata, product_name, plan_name, target
816809
)
817810
tech_config = res["tech_config"]
818-
tgt = res["target"]
819811
# We don't want to seek for SAS anymore as it was already found
820812
if res["sas_found"]:
821813
break
@@ -827,15 +819,15 @@ def publish(self, metadata: AzurePublishingMetadata) -> None:
827819
log.info(
828820
"Scanning the disk versions from \"%s\" on \"%s\" for the image \"%s\"",
829821
metadata.destination,
830-
tgt,
822+
target,
831823
metadata.image_path,
832824
)
833825
dv = seek_disk_version(tech_config, metadata.disk_version)
834826
disk_version = self._create_or_update_disk_version(res, source, dv)
835827

836828
# 4. With the updated disk_version we should adjust the SKUs and submit the changes
837829
if disk_version:
838-
log.info("Updating SKUs for \"%s\" on \"%s\".", metadata.destination, tgt)
830+
log.info("Updating SKUs for \"%s\" on \"%s\".", metadata.destination, target)
839831
tech_config.skus = update_skus(
840832
disk_versions=tech_config.disk_versions,
841833
generation=metadata.generation,
@@ -845,7 +837,7 @@ def publish(self, metadata: AzurePublishingMetadata) -> None:
845837
log.info(
846838
"Updating the technical configuration for \"%s\" on \"%s\".",
847839
metadata.destination,
848-
tgt,
840+
target,
849841
)
850842
self.configure(resources=[tech_config])
851843

@@ -865,7 +857,7 @@ def publish(self, metadata: AzurePublishingMetadata) -> None:
865857
log.info(
866858
"Publishing the new changes for \"%s\" on plan \"%s\"", product_name, plan_name
867859
)
868-
logdiff(self.diff_offer(product))
860+
logdiff(self.diff_offer(product, target))
869861
self.ensure_can_publish(product.id)
870862

871863
# According to the documentation we only need to pass the

0 commit comments

Comments
 (0)