Skip to content

Commit 89ffc7c

Browse files
authored
[gcp][feat] Save parsing with feedback (#2160)
1 parent 34d50ba commit 89ffc7c

File tree

7 files changed

+60
-27
lines changed

7 files changed

+60
-27
lines changed

plugins/azure/fix_plugin_azure/azure_client.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -325,10 +325,10 @@ def _list_with_retry(self, spec: MicrosoftRestSpec, **kwargs: Any) -> Optional[L
325325
raise MetricRequestError from e
326326
code = error.code or "Unknown"
327327
self.accumulator.add_error(False, code, spec.service, spec.action, str(e), self.location)
328-
log.warning(f"[Azure] Client Error: status={e.status_code}, error={e.error}, message={e}")
328+
log.warning(f"[Azure] Client Error: status={e.status_code}, error={e.error}, message={e}, spec={spec}")
329329
return None
330330
except Exception as e:
331-
log.warning(f"[Azure] called service={spec.service}: hit unexpected error: {e}", exc_info=e)
331+
log.warning(f"[Azure] called service={spec.service}: hit unexpected error: {e}, spec={spec}", exc_info=e)
332332
if self.config.discard_account_on_resource_error:
333333
raise
334334
return None

plugins/azure/fix_plugin_azure/resource/postgresql.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -550,7 +550,11 @@ def _collect_items(
550550
def post_process(self, graph_builder: GraphBuilder, source: Json) -> None:
551551
if server_id := self.id:
552552
resources_to_collect = [
553-
("administrators", AzurePostgresqlServerADAdministrator, ["InternalServerError"]),
553+
(
554+
"administrators",
555+
AzurePostgresqlServerADAdministrator,
556+
["InternalServerError", "DatabaseDoesNotExist"],
557+
),
554558
("configurations", AzurePostgresqlServerConfiguration, ["ServerStoppedError", "InternalServerError"]),
555559
("databases", AzurePostgresqlServerDatabase, ["ServerStoppedError", "InternalServerError"]),
556560
("firewallRules", AzurePostgresqlServerFirewallRule, None),

plugins/azure/fix_plugin_azure/resource/sql_server.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,11 @@ def fetch_data_encryption_status(sid: str) -> None:
268268
graph_builder.submit_work(service_name, fetch_data_encryption_status, database_id)
269269
resources_to_collect = [
270270
("geoBackupPolicies", AzureSqlServerDatabaseGeoBackupPolicy, None),
271-
("advisors?$expand=recommendedAction", AzureSqlServerAdvisor, None),
271+
(
272+
"advisors?$expand=recommendedAction",
273+
AzureSqlServerAdvisor,
274+
["DataWarehouseNotSupported", "DatabaseDoesNotExist"],
275+
),
272276
("workloadGroups", AzureSqlServerDatabaseWorkloadGroup, ["FeatureDisabledOnSelectedEdition"]),
273277
]
274278

plugins/gcp/fix_plugin_gcp/resources/base.py

Lines changed: 37 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,28 @@ def get_client(resource: BaseResource) -> GcpClient:
4747
)
4848

4949

50+
def parse_json(
51+
json: Json, clazz: Type[T], builder: Optional[GraphBuilder] = None, mapping: Optional[Dict[str, Bender]] = None
52+
) -> Optional[T]:
53+
"""
54+
Use this method to parse json into a class. If the json can not be parsed, the error is reported to the core.
55+
Based on configuration, either the exception is raised or None is returned.
56+
:param json: the json to parse.
57+
:param clazz: the class to parse into.
58+
:param builder: the graph builder.
59+
:param mapping: the optional mapping to apply before parsing.
60+
:return: The parsed object or None.
61+
"""
62+
try:
63+
mapped = bend(mapping, json) if mapping is not None else json
64+
return from_js(mapped, clazz)
65+
except Exception as e:
66+
if builder:
67+
# report and log the error
68+
builder.core_feedback.error(f"Failed to parse json into {clazz.__name__}: {e}. Source: {json}", log)
69+
return None
70+
71+
5072
class GraphBuilder:
5173
def __init__(
5274
self,
@@ -379,24 +401,25 @@ def collect(cls: Type[GcpResource], raw: List[Json], builder: GraphBuilder) -> L
379401
result: List[GcpResource] = []
380402
for js in raw:
381403
# map from api
382-
instance = cls.from_api(js)
383-
# allow the instance to adjust itself
384-
adjusted = instance.adjust_from_api(builder, js)
385-
# add to graph
386-
if (added := builder.add_node(adjusted, js)) is not None:
387-
# post process
388-
added.post_process(builder, js)
389-
result.append(added)
404+
if instance := cls.from_api(js, builder):
405+
# allow the instance to adjust itself
406+
adjusted = instance.adjust_from_api(builder, js)
407+
# add to graph
408+
if (added := builder.add_node(adjusted, js)) is not None:
409+
# post process
410+
added.post_process(builder, js)
411+
result.append(added)
390412
return result
391413

392414
@classmethod
393415
def from_json(cls: Type[GcpResourceType], json: Json) -> GcpResourceType:
394416
return from_js(json, cls)
395417

396418
@classmethod
397-
def from_api(cls: Type[GcpResourceType], json: Json) -> GcpResourceType:
398-
mapped = bend(cls.mapping, json)
399-
return cls.from_json(mapped)
419+
def from_api(
420+
cls: Type[GcpResourceType], json: Json, builder: Optional[GraphBuilder] = None
421+
) -> Optional[GcpResourceType]:
422+
return parse_json(json, cls, builder, cls.mapping)
400423

401424
@classmethod
402425
def called_collect_apis(cls) -> List[GcpApiSpec]:
@@ -541,9 +564,9 @@ def fallback_global_region(cls: Type[GcpRegion], project: GcpProject) -> GcpRegi
541564
return cls(id="global", tags={}, name="global", account=project)
542565

543566
def post_process(self, graph_builder: GraphBuilder, source: Json) -> None:
544-
region_quota = GcpRegionQuota.from_api(source)
545-
graph_builder.add_node(region_quota, source)
546-
graph_builder.add_edge(self, node=region_quota)
567+
if region_quota := GcpRegionQuota.from_api(source, graph_builder):
568+
graph_builder.add_node(region_quota, source)
569+
graph_builder.add_edge(self, node=region_quota)
547570

548571
def connect_in_graph(self, builder: GraphBuilder, source: Json) -> None:
549572
super().connect_in_graph(builder, source)

plugins/gcp/fix_plugin_gcp/resources/compute.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4217,8 +4217,8 @@ def collect_individual(cls: Type[GcpResource], builder: GraphBuilder, zone: str,
42174217
machineType=name,
42184218
)
42194219
result[InternalZoneProp] = zone # `add_node()` picks this up and sets proper zone/region
4220-
machine_type_obj = GcpMachineType.from_api(result)
4221-
builder.add_node(machine_type_obj, result)
4220+
if machine_type_obj := GcpMachineType.from_api(result, builder):
4221+
builder.add_node(machine_type_obj, result)
42224222

42234223
def _machine_type_matches_sku_description(self, sku_description: str) -> bool:
42244224
if not self.name:

plugins/gcp/fix_plugin_gcp/resources/storage.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -422,12 +422,12 @@ def pre_delete(self, graph: Graph) -> bool:
422422
client = get_client(self)
423423
objects = client.list(GcpObject.api_spec, bucket=self.name)
424424
for obj in objects:
425-
object_in_bucket = GcpObject.from_api(obj)
426-
client.delete(
427-
object_in_bucket.api_spec.for_delete(),
428-
bucket=self.name,
429-
resource=object_in_bucket.name,
430-
)
425+
if object_in_bucket := GcpObject.from_api(obj):
426+
client.delete(
427+
object_in_bucket.api_spec.for_delete(),
428+
bucket=self.name,
429+
resource=object_in_bucket.name,
430+
)
431431
return True
432432

433433
def delete(self, graph: Graph) -> bool:

plugins/gcp/test/random_client.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,9 @@ def create_node_for(
241241
result = client.list(api_spec=spec)
242242
assert len(result) > 0
243243
raw = adapt(result[0])
244-
return raw, clazz.from_api(raw)
244+
instance = clazz.from_api(raw)
245+
assert instance is not None
246+
return raw, instance
245247

246248

247249
def create_node(clazz: Type[GcpResourceType], **kwargs: Any) -> Tuple[Json, GcpResourceType]:

0 commit comments

Comments
 (0)