-
Notifications
You must be signed in to change notification settings - Fork 22
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.
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/integration iqair devices #3993
Update/integration iqair devices #3993
Conversation
Updates from airqo staging
📝 WalkthroughWalkthroughThe changes in this pull request primarily involve modifications to various methods across multiple classes in the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
src/workflows/airqo_etl_utils/airqo_api.py (2)
Line range hint
171-181
: Review Error Handling inget_devices
MethodIn the exception handling block of the
get_devices
method, an empty list is returned if an error occurs. This might mask underlying issues. Consider raising an exception or providing more contextual information to aid in debugging.
Line range hint
205-219
: Addnet_status
Parameter Documentation inget_networks
MethodThe
get_networks
method now includes anet_status
parameter. To enhance code clarity, include this parameter in the method's docstring along with its purpose and possible values.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (8)
src/workflows/airqo_etl_utils/airqo_api.py
(1 hunks)src/workflows/airqo_etl_utils/airqo_utils.py
(4 hunks)src/workflows/airqo_etl_utils/bigquery_api.py
(6 hunks)src/workflows/airqo_etl_utils/daily_data_utils.py
(0 hunks)src/workflows/airqo_etl_utils/data_warehouse_utils.py
(0 hunks)src/workflows/airqo_etl_utils/schema/raw_measurements.json
(1 hunks)src/workflows/airqo_etl_utils/weather_data_utils.py
(0 hunks)src/workflows/dags/airqo_measurements.py
(1 hunks)
💤 Files with no reviewable changes (3)
- src/workflows/airqo_etl_utils/weather_data_utils.py
- src/workflows/airqo_etl_utils/data_warehouse_utils.py
- src/workflows/airqo_etl_utils/daily_data_utils.py
🔇 Additional comments (9)
src/workflows/airqo_etl_utils/airqo_api.py (1)
Line range hint 138-203
: Ensure Proper Handling of the network
Parameter in get_devices
Method
The get_devices
method now accepts a network
parameter to retrieve devices based on the network. Verify that all calls to this method pass the correct network
value. Additionally, consider handling cases where network
might be None
to ensure backward compatibility and prevent potential errors.
src/workflows/airqo_etl_utils/schema/raw_measurements.json (1)
7-11
: Addition of network
Field to Schema
The network
field has been successfully added to the schema with the appropriate type and mode. Ensure that this field is correctly populated during data ingestion and that downstream processes are updated to accommodate this new field.
src/workflows/dags/airqo_measurements.py (1)
45-46
: Inclusion of network
Parameter in extract_aggregated_raw_data
The network
parameter has been added to the extract_aggregated_raw_data
function call with the value "airqo"
. Confirm that the function definition in AirQoDataUtils
accepts this parameter and that it is used appropriately within the function to filter data by network.
src/workflows/airqo_etl_utils/bigquery_api.py (2)
512-512
: Update compose_query
Method to Include network
Parameter
The compose_query
method signature now includes the network
parameter with a default value "all"
. Verify that all calls to this method pass the correct network
value and that the method handles cases when network
is "all"
appropriately in the query construction.
Line range hint 614-654
: Ensure Proper Handling of Optional network
Parameter in query_data
Method
The query_data
method now accepts an optional network
parameter. Review the method's logic to ensure it correctly handles cases when network
is None
or specified. Additionally, confirm that the dynamic query generation accommodates the network
filter appropriately.
src/workflows/airqo_etl_utils/airqo_utils.py (4)
62-62
: Update Call to bigquery_api.query_data
with network
Parameter
In the extract_uncalibrated_data
method, the call to bigquery_api.query_data
now includes the network
parameter set to str(Tenant.AIRQO)
. Ensure that this aligns with the updated method signature and that the correct network data is retrieved.
82-82
: Inclusion of network
Parameter in extract_data_from_bigquery
The network
parameter is added to the bigquery_api.query_data
call within extract_data_from_bigquery
. Verify that this parameter is correctly propagated and that data is filtered based on the specified network.
Line range hint 120-134
: Enhance Flexibility with network
Parameter in extract_aggregated_raw_data
The extract_aggregated_raw_data
method now accepts an optional network
parameter. This enhances data extraction flexibility. Ensure that when network
is None
, the method behaves as expected, possibly retrieving data across all networks.
Line range hint 205-210
: Improve Exception Handling in flatten_field_8
Method
The exception handling in flatten_field_8
has been refined to provide clearer logging messages. This aids in debugging but ensure that sensitive information is not logged. Consider adding more specific exception types if possible.
where_clause = f" timestamp between '{start_date_time}' and '{end_date_time}' " | ||
|
||
if network: | ||
where_clause += f"AND network = '{network}' " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Validate SQL Query Construction for network
Filter
The inclusion of the network
filter in the where_clause
should be carefully constructed to prevent SQL injection vulnerabilities. Ensure that the network
variable is sanitized or parameterized within the query to enhance security.
Description
Updates to include network in the bigquery query builder
Summary by CodeRabbit
Release Notes
New Features
network
field added to the raw measurements schema for improved data tracking.Improvements
Bug Fixes
These changes aim to provide users with a more robust and flexible data extraction experience, enhancing the overall functionality of the application.