Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update fix/analytics data export cleanup #4300

Merged
merged 3 commits into from
Jan 28, 2025
Merged

Update fix/analytics data export cleanup #4300

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a1d0c6a
Clean up tables and tenant/network
NicholasTurner23 Jan 28, 2025
46c5d22
Remove redundant code.
NicholasTurner23 Jan 28, 2025
be197c9
Update filter non private sites/devices implementation
NicholasTurner23 Jan 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 23 additions & 27 deletions src/analytics/api/models/events.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,8 @@ class EventsModel(BasePyMongoModel):
BIGQUERY_GRIDS_SITES = f"`{CONFIGURATIONS.BIGQUERY_GRIDS_SITES}`"
BIGQUERY_COHORTS = f"`{CONFIGURATIONS.BIGQUERY_COHORTS}`"
BIGQUERY_COHORTS_DEVICES = f"`{CONFIGURATIONS.BIGQUERY_COHORTS_DEVICES}`"
BIGQUERY_SITES = f"`{CONFIGURATIONS.BIGQUERY_SITES}`"
BIGQUERY_SITES_SITES = f"`{CONFIGURATIONS.BIGQUERY_SITES_SITES}`"
BIGQUERY_DEVICES = f"`{CONFIGURATIONS.BIGQUERY_DEVICES}`"
BIGQUERY_DEVICES_DEVICES = f"`{CONFIGURATIONS.BIGQUERY_DEVICES_DEVICES}`"
BIGQUERY_SITES = f"`{CONFIGURATIONS.BIGQUERY_SITES_SITES}`"
BIGQUERY_DEVICES = f"`{CONFIGURATIONS.BIGQUERY_DEVICES_DEVICES}`"
DATA_EXPORT_DECIMAL_PLACES = CONFIGURATIONS.DATA_EXPORT_DECIMAL_PLACES

BIGQUERY_EVENTS = CONFIGURATIONS.BIGQUERY_EVENTS
Expand All @@ -43,40 +41,38 @@ class EventsModel(BasePyMongoModel):

DEVICES_SUMMARY_TABLE = CONFIGURATIONS.DEVICES_SUMMARY_TABLE

def __init__(self, tenant):
def __init__(self, network):
"""
Initializes the EventsModel with default settings and mappings for limit thresholds,
and specifies collections and BigQuery table references.

Args:
tenant (str): The tenant identifier for managing database collections.
network (str): The network identifier for managing database collections.
"""
self.limit_mapper = {"pm2_5": 500.5, "pm10": 604.5, "no2": 2049}
self.sites_table = self.BIGQUERY_SITES
self.sites_sites_table = self.BIGQUERY_SITES_SITES
self.airqlouds_sites_table = self.BIGQUERY_AIRQLOUDS_SITES
self.devices_table = self.BIGQUERY_DEVICES
self.devices_devices_table = self.BIGQUERY_DEVICES_DEVICES
self.airqlouds_table = self.BIGQUERY_AIRQLOUDS
super().__init__(tenant, collection_name="events")
super().__init__(network, collection_name="events")

@property
def device_info_query(self):
"""Generates a device information query including site_id, network, and approximate location details."""
return (
f"{self.devices_devices_table}.site_id AS site_id, "
f"{self.devices_devices_table}.network AS network "
f"{self.devices_table}.site_id AS site_id, "
f"{self.devices_table}.network AS network "
)

@property
def device_info_query_airqloud(self):
"""Generates a device information query specifically for airqlouds, excluding the site_id."""
return f"{self.devices_devices_table}.network AS network "
return f"{self.devices_table}.network AS network "

@property
def site_info_query(self):
"""Generates a site information query to retrieve site name and approximate location details."""
return f"{self.sites_sites_table}.name AS site_name "
return f"{self.sites_table}.name AS site_name "

@property
def airqloud_info_query(self):
Expand All @@ -96,8 +92,8 @@ def add_device_join(self, data_query, filter_clause=""):
"""
return (
f"SELECT {self.device_info_query}, data.* "
f"FROM {self.devices_devices_table} "
f"RIGHT JOIN ({data_query}) data ON data.device_name = {self.devices_devices_table}.device_id "
f"FROM {self.devices_table} "
f"RIGHT JOIN ({data_query}) data ON data.device_name = {self.devices_table}.device_id "
f"{filter_clause}"
)

Expand All @@ -114,8 +110,8 @@ def add_device_join_to_airqlouds(self, data_query, filter_clause=""):
"""
return (
f"SELECT {self.device_info_query_airqloud}, data.* "
f"FROM {self.devices_devices_table} "
f"RIGHT JOIN ({data_query}) data ON data.site_id = {self.devices_devices_table}.site_id "
f"FROM {self.devices_table} "
f"RIGHT JOIN ({data_query}) data ON data.site_id = {self.devices_table}.site_id "
f"{filter_clause}"
)

Expand All @@ -131,8 +127,8 @@ def add_site_join(self, data_query):
"""
return (
f"SELECT {self.site_info_query}, data.* "
f"FROM {self.sites_sites_table} "
f"RIGHT JOIN ({data_query}) data ON data.site_id = {self.sites_sites_table}.id "
f"FROM {self.sites_table} "
f"RIGHT JOIN ({data_query}) data ON data.site_id = {self.sites_table}.id "
)

def add_airqloud_join(self, data_query):
Expand Down Expand Up @@ -198,23 +194,23 @@ def get_device_query(
including BAM data if applicable.
"""
query = (
f"{pollutants_query}, {time_grouping}, {self.device_info_query}, {self.devices_devices_table}.name AS device_name "
f"{pollutants_query}, {time_grouping}, {self.device_info_query}, {self.devices_table}.name AS device_name "
f"FROM {data_table} "
f"JOIN {self.devices_devices_table} ON {self.devices_devices_table}.device_id = {data_table}.device_id "
f"JOIN {self.devices_table} ON {self.devices_table}.device_id = {data_table}.device_id "
f"WHERE {data_table}.timestamp BETWEEN '{start_date}' AND '{end_date}' "
f"AND {self.devices_devices_table}.device_id IN UNNEST(@filter_value) "
f"AND {self.devices_table}.device_id IN UNNEST(@filter_value) "
)
if frequency in ["weekly", "monthly", "yearly"]:
query += " GROUP BY ALL"

query = self.add_site_join(query)
if frequency in ["hourly", "weekly", "monthly", "yearly"]:
bam_query = (
f"{bam_pollutants_query}, {time_grouping}, {self.device_info_query}, {self.devices_devices_table}.name AS device_name "
f"{bam_pollutants_query}, {time_grouping}, {self.device_info_query}, {self.devices_table}.name AS device_name "
f"FROM {self.BIGQUERY_BAM_DATA} "
f"JOIN {self.devices_devices_table} ON {self.devices_devices_table}.device_id = {self.BIGQUERY_BAM_DATA}.device_id "
f"JOIN {self.devices_table} ON {self.devices_table}.device_id = {self.BIGQUERY_BAM_DATA}.device_id "
f"WHERE {self.BIGQUERY_BAM_DATA}.timestamp BETWEEN '{start_date}' AND '{end_date}' "
f"AND {self.devices_devices_table}.device_id IN UNNEST(@filter_value) "
f"AND {self.devices_table}.device_id IN UNNEST(@filter_value) "
)
if frequency in ["weekly", "monthly", "yearly"]:
bam_query += " GROUP BY ALL"
Expand Down Expand Up @@ -251,9 +247,9 @@ def get_site_query(
query = (
f"{pollutants_query}, {time_grouping}, {self.site_info_query}, {data_table}.device_id AS device_name "
f"FROM {data_table} "
f"JOIN {self.sites_sites_table} ON {self.sites_sites_table}.id = {data_table}.site_id "
f"JOIN {self.sites_table} ON {self.sites_table}.id = {data_table}.site_id "
f"WHERE {data_table}.timestamp BETWEEN '{start_date}' AND '{end_date}' "
f"AND {self.sites_sites_table}.id IN UNNEST(@filter_value) "
f"AND {self.sites_table}.id IN UNNEST(@filter_value) "
)
if frequency in ["weekly", "monthly", "yearly"]:
query += " GROUP BY ALL"
Expand Down
96 changes: 21 additions & 75 deletions src/analytics/api/utils/data_formatters.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ def format_to_aqcsv(
"pm2_5_raw_value",
"pm10_raw_value",
"device_name",
"tenant",
"network",
"device_latitude",
"device_longitude",
"frequency",
Expand All @@ -265,69 +265,6 @@ def format_to_aqcsv(
return dataframe.to_dict("records")


def tenant_to_str(tenant: str) -> str:
try:
if tenant.lower() == "airqo":
return "AirQo"
elif tenant.lower() == "kcca":
return "KCCA"
elif tenant.lower() == "us_embassy":
return "US Embassy"
else:
pass
except Exception as ex:
pass

return ""


def device_category_to_str(device_category: str) -> str:
try:
if device_category.lower() == "bam":
return "Reference Monitor"
elif device_category.lower() == "lowcost":
return "Low Cost Sensor"
else:
pass
except Exception as ex:
pass

return ""


def filter_non_private_sites(filter_type: str, sites: List[str]) -> Dict[str, Any]:
"""
Filters out private site IDs from a provided array of site IDs.

Args:
sites(List[str]): List of site ids to filter against.

Returns:
a response dictionary object that contains a list of non-private site ids if any.
"""

if len(sites) == 0:
return {}

endpoint: str = "devices/grids/filterNonPrivateSites"

try:
airqo_requests = AirQoRequests()
response = airqo_requests.request(
endpoint=endpoint, body={filter_type: sites}, method="post"
)
if response and response.get("status") == "success":
return airqo_requests.create_response(
message="Successfully returned data.",
data=response.get("data"),
success=True,
)
else:
return airqo_requests.create_response(response, success=False)
except Exception as rex:
logger.exception(f"Error while filtering non private devices {rex}")


def validate_network(network_name: str) -> bool:
"""
Validate if a given network name exists in the list of networks.
Expand All @@ -353,33 +290,42 @@ def validate_network(network_name: str) -> bool:
return False


def filter_non_private_devices(filter_type: str, devices: List[str]) -> Dict[str, Any]:
def filter_non_private_sites_devices(
filter_type: str, filter_value: List[str]
) -> Dict[str, Any]:
"""
FilterS out private device IDs from a provided array of device IDs.
Filters out private device/site IDs from a provided array of device IDs.

Args:
entities(List[str]): List of device/site ids to filter against.
filter_type(str): A string either devices or sites.
filter_value(List[str]): List of device/site ids to filter against.

Returns:
a response dictionary object that contains a list of non-private device ids if any.
A response dictionary object that contains a list of non-private device/site ids if any.
"""

if len(devices) == 0:
return {}
if len(filter_value) == 0:
raise ValueError(f"{filter_type} can't be empty")

endpoint: Dict = {
"devices": "devices/cohorts/filterNonPrivateDevices",
"sites": "devices/grids/filterNonPrivateSites",
}

endpoint: str = "devices/cohorts/filterNonPrivateDevices"
try:
airqo_requests = AirQoRequests()
response = airqo_requests.request(
endpoint=endpoint, body={filter_type: devices}, method="post"
endpoint=endpoint.get(filter_type),
body={filter_type: filter_value},
method="post",
)
if response and response.get("status") == "success":
return airqo_requests.create_response(
message="Successfully returned data.",
data=response.get("data"),
data=response.get("data", {}).get(filter_type, []),
success=True,
)
else:
return airqo_requests.create_response(response, success=False)
except Exception as rex:
logger.exception(f"Error while filtering non private devices {rex}")
except Exception as e:
logger.exception(f"Error while filtering non private {filter_type}: {e}")
Comment on lines +307 to +331
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance input validation and error handling.

The implementation could be more robust with additional validations and consistent error handling:

+    ALLOWED_FILTER_TYPES = {"devices", "sites"}
+
+    if filter_type not in ALLOWED_FILTER_TYPES:
+        raise ValueError(f"filter_type must be one of {ALLOWED_FILTER_TYPES}")
+
     if len(filter_value) == 0:
-        raise ValueError(f"{filter_type} can't be empty")
+        raise ValueError(f"filter_value for {filter_type} cannot be empty")

     endpoint: Dict = {
         "devices": "devices/cohorts/filterNonPrivateDevices",
         "sites": "devices/grids/filterNonPrivateSites",
     }

     try:
         airqo_requests = AirQoRequests()
         response = airqo_requests.request(
             endpoint=endpoint.get(filter_type),
             body={filter_type: filter_value},
             method="post",
         )
         if response and response.get("status") == "success":
             return airqo_requests.create_response(
                 message="Successfully returned data.",
                 data=response.get("data", {}).get(filter_type, []),
                 success=True,
             )
-        else:
-            return airqo_requests.create_response(response, success=False)
+        return airqo_requests.create_response(
+            message=f"Failed to filter {filter_type}. Server response: {response}",
+            success=False
+        )
     except Exception as e:
-        logger.exception(f"Error while filtering non private {filter_type}: {e}")
+        error_msg = f"Error while filtering non-private {filter_type}: {str(e)}"
+        logger.exception(error_msg)
+        return airqo_requests.create_response(
+            message=error_msg,
+            success=False
+        )
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if len(filter_value) == 0:
raise ValueError(f"{filter_type} can't be empty")
endpoint: Dict = {
"devices": "devices/cohorts/filterNonPrivateDevices",
"sites": "devices/grids/filterNonPrivateSites",
}
endpoint: str = "devices/cohorts/filterNonPrivateDevices"
try:
airqo_requests = AirQoRequests()
response = airqo_requests.request(
endpoint=endpoint, body={filter_type: devices}, method="post"
endpoint=endpoint.get(filter_type),
body={filter_type: filter_value},
method="post",
)
if response and response.get("status") == "success":
return airqo_requests.create_response(
message="Successfully returned data.",
data=response.get("data"),
data=response.get("data", {}).get(filter_type, []),
success=True,
)
else:
return airqo_requests.create_response(response, success=False)
except Exception as rex:
logger.exception(f"Error while filtering non private devices {rex}")
except Exception as e:
logger.exception(f"Error while filtering non private {filter_type}: {e}")
ALLOWED_FILTER_TYPES = {"devices", "sites"}
if filter_type not in ALLOWED_FILTER_TYPES:
raise ValueError(f"filter_type must be one of {ALLOWED_FILTER_TYPES}")
if len(filter_value) == 0:
raise ValueError(f"filter_value for {filter_type} cannot be empty")
endpoint: Dict = {
"devices": "devices/cohorts/filterNonPrivateDevices",
"sites": "devices/grids/filterNonPrivateSites",
}
try:
airqo_requests = AirQoRequests()
response = airqo_requests.request(
endpoint=endpoint.get(filter_type),
body={filter_type: filter_value},
method="post",
)
if response and response.get("status") == "success":
return airqo_requests.create_response(
message="Successfully returned data.",
data=response.get("data", {}).get(filter_type, []),
success=True,
)
return airqo_requests.create_response(
message=f"Failed to filter {filter_type}. Server response: {response}",
success=False
)
except Exception as e:
error_msg = f"Error while filtering non-private {filter_type}: {str(e)}"
logger.exception(error_msg)
return airqo_requests.create_response(
message=error_msg,
success=False
)

58 changes: 28 additions & 30 deletions src/analytics/api/views/dashboard.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,7 @@
SiteModel,
ExceedanceModel,
)
from api.utils.data_formatters import (
filter_non_private_sites,
filter_non_private_devices,
)
from api.utils.data_formatters import filter_non_private_sites_devices

# Middlewares
from api.utils.http import AirQoRequests
Expand Down Expand Up @@ -45,12 +42,12 @@ class ChartDataResource(Resource):
"chartType|required:str",
)
def post(self):
tenant = request.args.get("tenant", "airqo")
network = request.args.get("network", "airqo")

json_data = request.get_json()
sites = filter_non_private_sites(sites=json_data.get("sites", {})).get(
"sites", []
)
sites = filter_non_private_sites_devices(
filter_type="sites", filter_value=json_data.get("sites", {})
).get("data", [])
Comment on lines +48 to +50
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix the default value for sites filter.

Using an empty dict as the default value for json_data.get("sites", {}) is incorrect as the function expects a list:

-        sites = filter_non_private_sites_devices(
-            filter_type="sites", filter_value=json_data.get("sites", {})
-        ).get("data", [])
+        sites = filter_non_private_sites_devices(
+            filter_type="sites", filter_value=json_data.get("sites", [])
+        ).get("data", [])
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
sites = filter_non_private_sites_devices(
filter_type="sites", filter_value=json_data.get("sites", {})
).get("data", [])
sites = filter_non_private_sites_devices(
filter_type="sites", filter_value=json_data.get("sites", [])
).get("data", [])

start_date = json_data["startDate"]
end_date = json_data["endDate"]
frequency = json_data["frequency"]
Expand All @@ -59,11 +56,10 @@ def post(self):

colors = ["#7F7F7F", "#E377C2", "#17BECF", "#BCBD22", "#3f51b5"]

events_model = EventsModel(tenant)
events_model = EventsModel(network)
data = events_model.get_chart_events(
sites, start_date, end_date, pollutant, frequency
)

chart_datasets = []
chart_labels = []

Expand Down Expand Up @@ -189,17 +185,18 @@ def _get_validated_filter(self, json_data):
filter_value = json_data.get(filter_type)

if filter_type in sites:
validated_value = filter_non_private_sites(filter_type, filter_value)
validated_value = filter_non_private_sites_devices(
filter_type, filter_value
)
elif filter_type in devices:
validated_value = filter_non_private_devices(filter_type, filter_value)
validated_value = filter_non_private_sites_devices(
filter_type, filter_value
)
Comment on lines +188 to +194
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add filter type validation for device registry calls.

The filter_type is passed directly from the provided_filters list without ensuring it matches the expected values ("devices" or "sites"):

+        def normalize_filter_type(filter_type: str) -> str:
+            if filter_type in sites:
+                return "sites"
+            if filter_type in devices:
+                return "devices"
+            return filter_type

         if filter_type in sites:
             validated_value = filter_non_private_sites_devices(
-                filter_type, filter_value
+                normalize_filter_type(filter_type), filter_value
             )
         elif filter_type in devices:
             validated_value = filter_non_private_sites_devices(
-                filter_type, filter_value
+                normalize_filter_type(filter_type), filter_value
             )

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 Ruff (0.8.2)

187-194: Combine if branches using logical or operator

Combine if branches

(SIM114)

else:
return filter_type, filter_value, None

if validated_value and validated_value.get("status") == "success":
# TODO This should be cleaned up.
validated_data = validated_value.get("data", {}).get(
"sites" if filter_type in sites else "devices", []
)
validated_data = validated_value.get("data", [])
else:
error_message = validated_value.get("message", "Validation failed")

Expand Down Expand Up @@ -274,9 +271,9 @@ def post(self):
pollutant = json_data["pollutant"]
start_date = json_data["startDate"]
end_date = json_data["endDate"]
sites = filter_non_private_sites(sites=json_data.get("sites", {})).get(
"sites", []
)
sites = filter_non_private_sites_devices(
filter_type="sites", filter_value=json_data.get("sites", {})
).get("data", [])
Comment on lines +274 to +276
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix the default value for sites filter.

Using an empty dict as the default value for json_data.get("sites", {}) is incorrect as the function expects a list:

-        sites = filter_non_private_sites_devices(
-            filter_type="sites", filter_value=json_data.get("sites", {})
-        ).get("data", [])
+        sites = filter_non_private_sites_devices(
+            filter_type="sites", filter_value=json_data.get("sites", [])
+        ).get("data", [])
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
sites = filter_non_private_sites_devices(
filter_type="sites", filter_value=json_data.get("sites", {})
).get("data", [])
sites = filter_non_private_sites_devices(
filter_type="sites", filter_value=json_data.get("sites", [])
).get("data", [])


events_model = EventsModel(tenant)
site_model = SiteModel(tenant)
Expand Down Expand Up @@ -333,15 +330,16 @@ class DailyAveragesResource2(Resource):
"devices|optional:list",
)
def post(self):
tenant = request.args.get("tenant", "airqo")
network = request.args.get("network", "airqo")
json_data = request.get_json()
pollutant = json_data["pollutant"]
start_date = json_data["startDate"]
end_date = json_data["endDate"]
devices = filter_non_private_devices(devices=json_data.get("devices", {})).get(
"devices", []
)
events_model = EventsModel(tenant)
devices = filter_non_private_sites_devices(
filter_type="devices", filter_value=json_data.get("devices", {})
).get("data", [])
Comment on lines +338 to +340
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix the default value for devices filter.

Using an empty dict as the default value for json_data.get("devices", {}) is incorrect as the function expects a list:

-        devices = filter_non_private_sites_devices(
-            filter_type="devices", filter_value=json_data.get("devices", {})
-        ).get("data", [])
+        devices = filter_non_private_sites_devices(
+            filter_type="devices", filter_value=json_data.get("devices", [])
+        ).get("data", [])
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
devices = filter_non_private_sites_devices(
filter_type="devices", filter_value=json_data.get("devices", {})
).get("data", [])
devices = filter_non_private_sites_devices(
filter_type="devices", filter_value=json_data.get("devices", [])
).get("data", [])


events_model = EventsModel(network)
data = events_model.get_device_averages_from_bigquery(
start_date, end_date, pollutant, devices=devices
)
Expand Down Expand Up @@ -391,9 +389,9 @@ def post(self):
standard = json_data["standard"]
start_date = json_data["startDate"]
end_date = json_data["endDate"]
sites = filter_non_private_sites(sites=json_data.get("sites", {})).get(
"sites", []
)
sites = filter_non_private_sites_devices(
filter_type="sites", filter_value=json_data.get("sites", {})
).get("data", [])
Comment on lines +392 to +394
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix the default value for sites filter.

Using an empty dict as the default value for json_data.get("sites", {}) is incorrect as the function expects a list:

-        sites = filter_non_private_sites_devices(
-            filter_type="sites", filter_value=json_data.get("sites", {})
-        ).get("data", [])
+        sites = filter_non_private_sites_devices(
+            filter_type="sites", filter_value=json_data.get("sites", [])
+        ).get("data", [])
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
sites = filter_non_private_sites_devices(
filter_type="sites", filter_value=json_data.get("sites", {})
).get("data", [])
sites = filter_non_private_sites_devices(
filter_type="sites", filter_value=json_data.get("sites", [])
).get("data", [])


exc_model = ExceedanceModel(tenant)
data = exc_model.get_exceedances(
Expand Down Expand Up @@ -427,9 +425,9 @@ def post(self):
standard = json_data["standard"]
start_date = json_data["startDate"]
end_date = json_data["endDate"]
devices = filter_non_private_devices(devices=json_data.get("devices", {})).get(
"devices", []
)
devices = filter_non_private_sites_devices(
filter_type="devices", filter_value=json_data.get("devices", {})
).get("data", [])
Comment on lines +428 to +430
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix the default value for devices filter.

Using an empty dict as the default value for json_data.get("devices", {}) is incorrect as the function expects a list:

-        devices = filter_non_private_sites_devices(
-            filter_type="devices", filter_value=json_data.get("devices", {})
-        ).get("data", [])
+        devices = filter_non_private_sites_devices(
+            filter_type="devices", filter_value=json_data.get("devices", [])
+        ).get("data", [])
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
devices = filter_non_private_sites_devices(
filter_type="devices", filter_value=json_data.get("devices", {})
).get("data", [])
devices = filter_non_private_sites_devices(
filter_type="devices", filter_value=json_data.get("devices", [])
).get("data", [])


events_model = EventsModel(tenant)
data = events_model.get_device_readings_from_bigquery(
Expand Down
Loading
Loading