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.

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

feat: Add Usage Calculator module and fetch kWh tariff from Usage Calculator #101

Merged
merged 12 commits into from
Apr 17, 2024

Conversation

GuyKh
Copy link
Owner

@GuyKh GuyKh commented Apr 16, 2024

User description

What changes do you are proposing?

Add Usage Calculator module and fetch kWh tariff from Usage Calculator

How did you test these changes?

manually

Closing issues


Type

enhancement, bug_fix


Description

  • Enhanced HTTP request handling with improved debug logging and support for non-JSON POST requests.
  • Added new API endpoints and corresponding functions in the data module for CheckContract, EFS Messages, TenantIdentity, and Invoice PDF retrieval.
  • Extended the IEC client with methods for contract check, saving invoice PDFs, retrieving device details, and fetching EFS messages.
  • Introduced models for contract checks, device identification, EFS messages, invoice PDF requests, and usage calculator data.
  • Implemented a usage calculator with functionalities for data loading, device information retrieval, and consumption calculation.
  • Fixed a typo in the Contract model field name.

Changes walkthrough

Relevant files
Enhancement
14 files
commons.py
Enhance HTTP Request Handling and Debug Logging                   

iec_api/commons.py

  • Removed debug logging for HTTP GET and POST requests.
  • Added send_non_json_post_request function for sending POST requests
    with non-JSON responses.
  • Implemented debug logging for request start, chunk sent, and request
    end.
  • +45/-13 
    const.py
    Add New API Endpoints for Various Features                             

    iec_api/const.py

  • Added new API endpoints for CheckContract, EFS Messages,
    TenantIdentity, Invoice PDF, and Calculator Gadget.
  • +6/-1     
    data.py
    Implement Functions for New API Endpoints and Fix URL Usage

    iec_api/data.py

  • Implemented functions to interact with new API endpoints:
    CheckContract, EFS Messages, TenantIdentity, and Invoice PDF.
  • Fixed incorrect URL constant usage for billing invoices.
  • +101/-12
    iec_client.py
    Extend IEC Client with New Features and Debug Logging       

    iec_api/iec_client.py

  • Added methods for contract check, saving invoice PDF, device details
    retrieval, and EFS messages.
  • Enhanced session initialization with custom debug logging.
  • +126/-9 
    contract_check.py
    Add ContractCheck Model for Contract Verification Data     

    iec_api/models/contract_check.py

    • Added ContractCheck model to represent contract check data.
    +65/-0   
    device_identity.py
    Add Models for Device Identification Data                               

    iec_api/models/device_identity.py

  • Added DeviceIdentity and DeviceDetails models for device
    identification data.
  • +55/-0   
    efs.py
    Add Models for EFS Messages and Requests                                 

    iec_api/models/efs.py

    • Added models for EFS (Email, Fax, SMS) messages and requests.
    +82/-0   
    get_pdf.py
    Add GetPdfRequest Model for Invoice PDF Retrieval               

    iec_api/models/get_pdf.py

    • Added GetPdfRequest model for invoice PDF retrieval requests.
    +18/-0   
    static_data.py
    Add Functions for Usage Calculator and kWh Tariff Data Retrieval

    iec_api/static_data.py

    • Added functions to retrieve usage calculator and kWh tariff data.
    +19/-0   
    calculator.py
    Implement Usage Calculator with Data Loading and Consumption
    Calculation

    iec_api/usage_calculator/calculator.py

  • Implemented a usage calculator with data loading and consumption
    calculation functionalities.
  • +88/-0   
    consumption.py
    Add Consumption Model for Energy Consumption Data               

    iec_api/usage_calculator/consumption.py

  • Added Consumption model for representing energy consumption data.
  • +16/-0   
    electric_device.py
    Add ElectricDevice Model with Power Unit and Calculation Resolution
    Enums

    iec_api/usage_calculator/electric_device.py

  • Added ElectricDevice model with power unit and calculation resolution
    enums.
  • +33/-0   
    get_calculator_response.py
    Add GetCalculatorResponse Model for Usage Calculator API Response

    iec_api/usage_calculator/get_calculator_response.py

  • Added GetCalculatorResponse model for usage calculator API response.
  • +14/-0   
    rates.py
    Add Rates Model for Electricity Rates Data                             

    iec_api/usage_calculator/rates.py

    • Added Rates model for representing electricity rates data.
    +14/-0   
    Bug_fix
    1 files
    contract.py
    Fix Typo in Contract Model Field Name                                       

    iec_api/models/contract.py

    • Fixed typo in from_private_producer field name.
    +1/-1     

    PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    @GuyKh GuyKh added bug Something isn't working enhancement New feature or request labels Apr 16, 2024
    @GuyKh GuyKh self-assigned this Apr 16, 2024
    @GuyKh GuyKh linked an issue Apr 16, 2024 that may be closed by this pull request
    Copy link

    what-the-diff bot commented Apr 16, 2024

    PR Summary

    • Enrichment of utility functions in iec_api/commons.py
      Several functions for HTTP requests and responses were refined, reducing excessive logging and integrating 'aiohttp' for improved network interactions.

    • Unification of codebase in iec_api/commons.py
      Elimination of redundant implementations improved the efficiency of the code. New functionality for Non-JSON post requests was introduced.

    • Optimization in iec_api/data.py and iec_api/iec_client.py
      The libraries were revamped by removing unused imports and updating certain functions, enhancing the execution of these files.

    • Constants Update in iec_api/consts.py
      Several URLs for various services were updated to reflect their current accurate endpoints.

    • Improvements in iec_api/models
      A new data class was added for Contract Checking, along with its decoder. Unused imports from contract.py were also removed, enhancing the functionality.

    • Introduction of new data classes in device_identity.py, efs.py, and get_pdf.py
      These files define data models to handle device identity information, EFS service messages, and PDF requests. This adds a new dimension in the ability to handle more diverse types of data.

    • Creation of static_data.py
      This file was added to manage retrieval of static data from the IEC API, improving automation and data consistency.

    • Establishment of usage_calculator module
      This new module features classes for using a calculator for measuring usage and costs and managing data related to electric devices and the calculator's rates and responses. These new additions enrich the functionality by providing usage and cost calculations.

    Copy link

    PR Description updated to latest commit (576904d)

    Copy link

    codiumai-pr-agent-pro bot commented Apr 16, 2024

    PR Review

    (Review updated until commit 2ce7f0b)

    ⏱️ Estimated effort to review [1-5]

    4, due to the introduction of several new modules, extensive changes across multiple files, and the complexity of integrating new functionalities such as the usage calculator and handling of different data models. The changes impact core functionalities and require careful consideration of design patterns, error handling, and performance implications.

    🧪 Relevant tests

    No

    🔍 Possible issues

    Possible Bug: The removal of GET_KWH_TARIFF_URL import in iec_api/data.py without removing its usage might lead to runtime errors. Ensure all references are updated or removed.

    Performance Concern: Loading the entire usage calculator data upon each request without caching mechanism could lead to unnecessary processing and increased response times. Consider implementing a caching strategy.

    Error Handling: Lack of explicit error handling in new async functions could result in unhandled exceptions. It's important to ensure robust error handling to maintain application stability.

    🔒 Security concerns

    No

    Code feedback:
    relevant fileiec_api/commons.py
    suggestion      

    Consider implementing a fallback or default value for timeout to ensure the request always has a timeout value, preventing potential hanging requests. [important]

    relevant linetimeout = session.timeout

    relevant fileiec_api/data.py
    suggestion      

    Replace the removed GET_KWH_TARIFF_URL usage with the correct implementation to avoid runtime errors. If the functionality is moved, ensure all references are correctly updated. [important]

    relevant line- GET_KWH_TARIFF_URL,

    relevant fileiec_api/iec_client.py
    suggestion      

    Use async with statement for file operations to ensure the file is properly closed even if an error occurs during the file write operation. This change improves the reliability of file handling. [important]

    relevant lineasync with aiofiles.open(file_path, "wb") as f:

    relevant fileiec_api/usage_calculator/calculator.py
    suggestion      

    Implement caching for the usage calculator data to improve performance. This could be done by checking if the data is already loaded before fetching it again. [medium]

    relevant lineif not self.is_loaded:


    ✨ Review tool usage guide:

    Overview:
    The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.

    The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.

    • When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:
    /review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...
    
    [pr_reviewer]
    some_config1=...
    some_config2=...
    

    See the review usage page for a comprehensive guide on using this tool.

    Copy link

    PR Code Suggestions

    CategorySuggestions                                                                                                                                                       
    Best practice
    Use async with for file operations to ensure proper resource management.

    Consider using async with statement for opening and writing to the file to ensure proper
    resource management and file closure.

    iec_api/data.py [258-260]

    -f = open(file_path, "wb")
    -f.write(response_bytes)
    -f.close()
    +async with aiofiles.open(file_path, "wb") as f:
    +    await f.write(response_bytes)
     
    Use formatted strings for constructing service codes.

    Instead of manually appending strings to create the service code, use formatted strings
    for clarity and to reduce the risk of errors.

    iec_api/data.py [161]

    -service_code="EFS" + str(service_code).zfill(3)
    +service_code=f"EFS{service_code:03}"
     
    Use with statement for file operations to ensure proper closure.

    To ensure that the file is properly closed even in case of an error, use with statement
    for file operations in save_invoice_pdf_to_file method.

    iec_api/iec_client.py [258-260]

    -f = open(file_path, "wb")
    -f.write(response_bytes)
    -f.close()
    +with open(file_path, "wb") as f:
    +    f.write(response_bytes)
     
    Enhancement
    Add a check for None response before decoding to improve error handling.

    To improve error handling, consider checking if response is not None before decoding it in
    _post_response_with_descriptor function.

    iec_api/data.py [105]

    +if response is None:
    +    raise IECError("No response received")
     response_with_descriptor = decoder.decode(response)
     
    Add handling for ClientError in HTTP GET request functions.

    It's recommended to handle the ClientError exception in the send_get_request and
    send_non_json_get_request functions to catch any client-related errors during the HTTP
    request execution. This ensures that all potential errors from the aiohttp client are
    gracefully handled and can provide more descriptive error messages to the caller.

    iec_api/commons.py [107]

    -resp = await session.get(url=url, headers=headers, timeout=timeout)
    +try:
    +    resp = await session.get(url=url, headers=headers, timeout=timeout)
    +except ClientError as ex:
    +    raise IECError(-1, f"Failed to communicate with IEC API due to ClientError: {str(ex)}")
     
    Use aiohttp's TraceConfig for structured logging of HTTP events.

    Implement logging for HTTP request start, chunk sent, and request end events using
    aiohttp's TraceConfig in the send_get_request, send_non_json_get_request,
    send_post_request, and send_non_json_post_request functions. This approach provides a more
    structured and flexible way to log HTTP requests and responses, improving the
    maintainability and debuggability of the code.

    iec_api/commons.py [107]

    +trace_config = aiohttp.TraceConfig()
    +trace_config.on_request_start.append(on_request_start_debug)
    +trace_config.on_request_chunk_sent.append(on_request_chunk_sent_debug)
    +trace_config.on_request_end.append(on_request_end_debug)
    +session = ClientSession(trace_configs=[trace_config])
     resp = await session.get(url=url, headers=headers, timeout=timeout)
     
    Use snake_case for alias names in metadata for consistency.

    The alias names in the metadata should follow a consistent naming convention. Consider
    using snake_case for consistency with Python's style guide.

    iec_api/usage_calculator/get_calculator_response.py [13-14]

    -gadget_calculator_rates: Rates = field(metadata=field_options(alias="gadget_Calculator_Rates"))
    -electric_devices: list[ElectricDevice] = field(metadata=field_options(alias="electric_Devices"))
    +gadget_calculator_rates: Rates = field(metadata=field_options(alias="gadget_calculator_rates"))
    +electric_devices: list[ElectricDevice] = field(metadata=field_options(alias="electric_devices"))
     
    Add concurrency control to prevent race conditions when loading usage_calculator.

    Consider using a semaphore or another form of concurrency control to prevent race
    conditions during the initialization of usage_calculator.

    iec_api/static_data.py [11-12]

     if not usage_calculator.is_loaded:
    -    await usage_calculator.load_data(session)
    +    async with semaphore:
    +        if not usage_calculator.is_loaded:  # Double-checked locking
    +            await usage_calculator.load_data(session)
     
    Use Decimal for monetary values to improve precision.

    For better precision and consistency, consider using Decimal instead of float for monetary
    values such as home_rate, general_rate, and vat.

    iec_api/usage_calculator/rates.py [12-14]

    -home_rate: float = field(metadata=field_options(alias="homeRate"))
    -general_rate: float = field(metadata=field_options(alias="generalRate"))
    -vat: float = field(metadata=field_options(alias="vat"))
    +home_rate: Decimal = field(metadata=field_options(alias="homeRate"))
    +general_rate: Decimal = field(metadata=field_options(alias="generalRate"))
    +vat: Decimal = field(metadata=field_options(alias="vat"))
     
    Convert consumption and cost fields to calculated properties.

    The consumption and cost fields could be calculated properties instead of being stored as
    fields, to ensure they are always up to date with the current power, duration, and cost
    rates.

    iec_api/usage_calculator/consumption.py [15-16]

    -consumption: float
    -cost: float
    +@property
    +def consumption(self) -> float:
    +    return self.power * self.duration_in_hours  # Assuming duration_in_hours is a converted property of duration
     
    +@property
    +def cost(self) -> float:
    +    return self.consumption * cost_rate  # Assuming cost_rate is defined elsewhere
    +
    Maintainability
    Extract EFS request object creation to a separate function for clarity.

    For better maintainability and readability, consider extracting the logic for constructing
    EfsRequestSingleService and EfsRequestAllServices objects into separate functions or
    methods.

    iec_api/data.py [159-164]

    -if service_code:
    -    req = EfsRequestSingleService(
    -        contract_number=contract_id, process_type=1, service_code="EFS" + str(service_code).zfill(3)
    -    )
    -else:
    -    req = EfsRequestAllServices(contract_number=contract_id, process_type=1)
    +req = create_efs_request(contract_id, service_code)
     
    Bug
    Fix incorrect assignment of timeout parameter in POST request functions.

    For the send_post_request and send_non_json_post_request functions, it's advisable to
    correct the assignment of the timeout parameter to headers in the case where timeout is
    not provided. This appears to be a typo or copy-paste error. The correct behavior should
    ensure that timeout is used appropriately and not mistakenly assigned to headers.

    iec_api/commons.py [161]

     if not timeout:
    -    headers = session.timeout
    +    timeout = session.timeout
     
    Correct the VAT calculation in the consumption cost calculation.

    The calculation of the rate in the get_consumption_by_device_and_time method seems
    incorrect. It multiplies home_rate by vat, which doesn't align with the typical
    calculation of VAT. The correct approach should add VAT to the home_rate and then apply it
    to the consumption to calculate the cost.

    iec_api/usage_calculator/calculator.py [63]

    -rate = self.rates.home_rate * self.rates.vat
    +rate = self.rates.home_rate * (1 + self.rates.vat / 100)
     
    Fix typo in the alias for the average duration field in the ElectricDevice dataclass.

    There's a typo in the alias for average_duration_time_of_operation_in_minutes field in the
    ElectricDevice dataclass. It's currently misspelled as
    avarageDurationTimeOfOperationInMinutes. Correcting the spelling will ensure consistency
    and avoid potential confusion or errors in serialization/deserialization processes.

    iec_api/usage_calculator/electric_device.py [31-32]

     average_duration_time_of_operation_in_minutes: float = field(
    -    metadata=field_options(alias="avarageDurationTimeOfOperationInMinutes")
    +    metadata=field_options(alias="averageDurationTimeOfOperationInMinutes")
     )
     
    Correct a typo in the alias for from_private_producer.

    There's a typo in the alias for from_private_producer. It should match the intended string
    value.

    iec_api/models/contract.py [55]

    -from_private_producer: bool = field(metadata=field_options(alias="fromPativteProducer"))
    +from_private_producer: bool = field(metadata=field_options(alias="fromPrivateProducer"))
     

    ✨ Improve tool usage guide:

    Overview:
    The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.

    • When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:
    /improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...
    
    [pr_code_suggestions]
    some_config1=...
    some_config2=...
    

    See the improve usage page for a comprehensive guide on using this tool.

    Copy link

    CI Failure Feedback

    Action: Run Tests and Lint

    Failed stage: Running Lint [❌]

    Failure summary:

    The action failed for two main reasons:

  • A warning was issued during the installation of the project iec-api version 0.2.13 because the file
    /src/README.md could not be found. This indicates a missing file that was expected to exist in the
    project directory.
  • The primary cause of failure is a linting error detected by ruff. Specifically, the function
    get_invoice_pdf is redefined in iec_api/data.py at line 291, which was previously defined at line
    274. This redefinition of an unused function violates the linting rules, resulting in a failure.

  • Relevant error logs:
    1:  ##[group]Operating System
    2:  Ubuntu
    ...
    
    305:  #10 4.990 
    306:  #10 4.990 Writing lock file
    307:  #10 4.990 
    308:  #10 4.990 Installing the current project: iec-api (0.2.13)
    309:  #10 4.991 
    310:  #10 4.991 Warning: The current project could not be installed: [Errno 2] No such file or directory: '/src/README.md'
    311:  #10 4.991 If you do not want to install the current project use --no-root.
    312:  #10 4.991 If you want to use Poetry only for dependency management but not for packaging, you can disable package mode by setting package-mode = false in your pyproject.toml file.
    313:  #10 4.991 In a future version of Poetry this warning will become an error!
    ...
    
    324:  �[36;1mmake docker/lint�[0m
    325:  shell: /usr/bin/bash -e {0}
    326:  ##[endgroup]
    327:  docker-compose run "iec-api" poetry run ruff check .
    328:  Creating network "py-iec-api_default" with the default driver
    329:  Creating py-iec-api_iec-api_run ... 
    330:  Creating py-iec-api_iec-api_run ... done
    331:  iec_api/data.py:291:11: F811 Redefinition of unused `get_invoice_pdf` from line 274
    332:  Found 1 error.
    333:  1
    334:  make: *** [Makefile:41: docker/lint] Error 1
    335:  ##[error]Process completed with exit code 2.
    

    ✨ CI feedback usage guide:

    The CI feedback tool (/checks) automatically triggers when a PR has a failed check.
    The tool analyzes the failed checks and provides several feedbacks:

    • Failed stage
    • Failed test name
    • Failure summary
    • Relevant error logs

    In addition to being automatically triggered, the tool can also be invoked manually by commenting on a PR:

    /checks "https://github.com/{repo_name}/actions/runs/{run_number}/job/{job_number}"
    

    where {repo_name} is the name of the repository, {run_number} is the run number of the failed check, and {job_number} is the job number of the failed check.

    Configuration options

    • enable_auto_checks_feedback - if set to true, the tool will automatically provide feedback when a check is failed. Default is true.
    • excluded_checks_list - a list of checks to exclude from the feedback, for example: ["check1", "check2"]. Default is an empty list.
    • enable_help_text - if set to true, the tool will provide a help message with the feedback. Default is true.
    • persistent_comment - if set to true, the tool will overwrite a previous checks comment with the new feedback. Default is true.
    • final_update_message - if persistent_comment is true and updating a previous checks message, the tool will also create a new message: "Persistent checks updated to latest commit". Default is true.

    See more information about the checks tool in the docs.

    @GuyKh
    Copy link
    Owner Author

    GuyKh commented Apr 16, 2024

    /review

    Copy link

    Persistent review updated to latest commit 2ce7f0b

    @GuyKh GuyKh merged commit 67f8725 into main Apr 17, 2024
    1 check passed
    @GuyKh GuyKh deleted the usage-calculator branch April 17, 2024 14:07
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    bug_fix bug Something isn't working enhancement New feature or request Review effort [1-5]: 4
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    Base tariff on Gadget Calculator
    1 participant