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 TenantIdentity / DeviceDetails API #100

Merged
merged 2 commits into from
Apr 15, 2024
Merged

feat: add TenantIdentity / DeviceDetails API #100

merged 2 commits into from
Apr 15, 2024

Conversation

GuyKh
Copy link
Owner

@GuyKh GuyKh commented Apr 15, 2024

User description

What changes do you are proposing?

add TenantIdentity / DeviceDetails API

How did you test these changes?

manually

Closing issues


Type

enhancement


Description

  • Added a new API endpoint constant GET_TENANT_IDENTITY_URL for fetching tenant identity by device ID.
  • Implemented new async functions get_device_details and get_device_details_by_code in data.py to interact with the new API endpoint.
  • Extended the iec_client.py with methods to fetch device details by device ID and device code, enhancing the client's functionality.
  • Introduced new models DeviceDetails and DeviceIdentity in device_identity.py to represent the device details data structure, along with a decoder for API response handling.

Changes walkthrough

Relevant files
Enhancement
const.py
Add Tenant Identity API Endpoint Constant                               

iec_api/const.py

  • Added GET_TENANT_IDENTITY_URL constant for the Tenant/Identify API
    endpoint.
  • +1/-0     
    data.py
    Implement Device Details Fetching Functions                           

    iec_api/data.py

  • Imported GET_TENANT_IDENTITY_URL and DeviceDetails model.
  • Added get_device_details and get_device_details_by_code async
    functions to fetch device details.
  • +21/-0   
    iec_client.py
    Extend IEC Client with Device Details Methods                       

    iec_api/iec_client.py

  • Imported DeviceDetails model.
  • Added get_device_details_by_device_id and
    get_device_details_by_device_id_and_code methods to the client.
  • +30/-0   
    device_identity.py
    Define Device Identity Models and Decoder                               

    iec_api/models/device_identity.py

  • Defined DeviceDetails and DeviceIdentity data classes for device
    identity information.
  • Implemented decoder for the device identity response.
  • +55/-0   

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

    @GuyKh GuyKh added the enhancement New feature or request label Apr 15, 2024
    @GuyKh GuyKh self-assigned this Apr 15, 2024
    Copy link

    what-the-diff bot commented Apr 15, 2024

    PR Summary

    • Incorporation of a new URL Constant
      A new URL constant was created to retrieve tenant (the owner of a project environment) identity. This improvement allows the system to identify and manage data and user access more efficiently.

    • Introduction of Functions for Device Details
      New functions have been developed to fetch details of a device and to acquire device details using a unique code. This helps in quick reference of device statistics and data, enhancing system usability.

    • Inclusion of Device Identity Model
      A new model to specifically cater to device identity has been implemented. It facilitates better organization, rendering and manipulation of device-related data.

    • Addition of Decoder for Device Identity Model
      A decoder for the new device identity model has also been added. This allows for the interpretation of data within the model structure and enables seamless data conversion and analysis.

    • New Methods in IEC Client
      Novel methods have been integrated into the IEC (International Electrotechnical Commission) client to fetch device details. This boosts the ability to efficiently retrieve data from the comprehensive suite of standards for electronics and related technologies developed by the IEC.

    Copy link

    PR Description updated to latest commit (439c6d0)

    Copy link

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

    PR Review

    (Review updated until commit 2248902)

    ⏱️ Estimated effort to review [1-5]

    3, because the PR involves adding new API endpoints, implementing async functions, and introducing new models. The complexity of async operations and the handling of API responses require careful review to ensure correctness and efficiency.

    🧪 Relevant tests

    No

     Insights from user answers

    The user's answers clarified that the API might return multiple devices for the same device code, the API returns datetime objects, and to get a specific DeviceDetails, filtering by both device_id and device_code is necessary.

    🔍 Possible issues

    Possible Bug: The get_device_details_by_code function might return None if no devices match the device_code, which could lead to unhandled NoneType errors in the calling code.

    Data Handling: The API's ability to return multiple devices for the same device code might require additional handling in the application logic to ensure the correct device is processed.

    🔒 Security concerns

    No

    Code feedback:
    relevant fileiec_api/data.py
    suggestion      

    Consider adding error handling for the case when get_device_details returns None to avoid AttributeError when accessing the device_details attribute. [important]

    relevant linereturn device_identity.device_details if device_identity else None

    relevant fileiec_api/data.py
    suggestion      

    To improve efficiency, consider caching the results of get_device_details if the API's response time is significant and the data does not change frequently. [medium]

    relevant lineasync def get_device_details(session: ClientSession, token: JWT, device_id: str) -> Optional[List[DeviceDetails]]:

    relevant fileiec_api/models/device_identity.py
    suggestion      

    Ensure that the datetime format returned by the API matches the expected format in last_reading_date. Consider adding a custom decoder if necessary to handle different datetime formats. [important]

    relevant linelast_reading_date: datetime = field(metadata=field_options(alias="lastReadingDate"))

    relevant fileiec_api/iec_client.py
    suggestion      

    For get_device_details_by_device_id_and_code, add logging to capture when no matching device is found for the given device_code. This will help in debugging and operational monitoring. [medium]

    relevant linereturn await data.get_device_details_by_code(self._session, self._token, device_id, device_code)


    ✨ 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.

    @GuyKh
    Copy link
    Owner Author

    GuyKh commented Apr 15, 2024

    /reflect_and_review

    Copy link

    PR Code Suggestions

    CategorySuggestions                                                                                                                                                       
    Enhancement
    Use f-strings for URL construction.

    Consider using f-strings for building URLs to enhance readability and performance.

    iec_api/const.py [39]

    -GET_TENANT_IDENTITY_URL = IEC_API_BASE_URL + "Tenant/Identify/{device_id}"
    +GET_TENANT_IDENTITY_URL = f"{IEC_API_BASE_URL}Tenant/Identify/{{device_id}}"
     
    Use datetime type for date fields.

    For last_date in DeviceIdentity, consider using datetime type instead of str for
    consistency with last_reading_date in DeviceDetails and to facilitate date manipulations.

    iec_api/models/device_identity.py [51]

    -last_date: str = field(metadata=field_options(alias="lastDate"))
    +last_date: datetime = field(metadata=field_options(alias="lastDate", decoder=datetime.fromisoformat))
     
    Best practice
    Correct the return type annotation for consistency.

    The return type of get_device_details should be Optional[List[DeviceDetails]] instead of
    Optional[list[DeviceDetails]] for consistency with Python's type hinting standards.

    iec_api/data.py [229]

    -async def get_device_details(session: ClientSession, token: JWT, device_id: str) -> Optional[list[DeviceDetails]]:
    +async def get_device_details(session: ClientSession, token: JWT, device_id: str) -> Optional[List[DeviceDetails]]:
     
    Maintainability
    Improve variable naming for readability.

    Use a more descriptive variable name than res to improve code readability.

    iec_api/data.py [231]

    -res = await _get_response_with_descriptor(
    +response = await _get_response_with_descriptor(
     
    Correct the docstring to accurately describe the return value.

    The docstring for get_device_details_by_device_id_and_code incorrectly states it returns a
    list. It should state it returns a single DeviceDetails instance or None.

    iec_api/iec_client.py [315-322]

     """
    -Get a list of devices for the user
    +Get device details for a specific device by device ID and code
     ...
     Returns:
    -    DeviceDetails: List of device details or No
    +    DeviceDetails: Device details or None
     """
     

    ✨ 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

    Questions to better understand the PR:

    1. In the iec_api/data.py file, the get_device_details function fetches device details from the API. Is there any specific reason why the function returns a list of DeviceDetails objects instead of a single DeviceDetails object?
    2. The DeviceDetails class in iec_api/models/device_identity.py has a field last_reading_date of type datetime. Is the API guaranteed to always return this field in a format that can be parsed into a datetime object?
    3. Could you elaborate on the use case for the get_device_details_by_code function in iec_api/data.py? How is it expected to be used in the context of the application?

    Please respond to the questions above in the following format:

    /answer

    1. ...
    2. ...
      ...

    @GuyKh
    Copy link
    Owner Author

    GuyKh commented Apr 15, 2024

    /answer

    1. It seems that the API fetch function isn't one-to-one and may return multiple devices with the same device code
    2. The API returns datetime
    3. Following the first bullet, to get a specific DeviceDetails, need to filter by both device_id and device_code

    Copy link

    Persistent review updated to latest commit 2248902

    @GuyKh GuyKh merged commit 95c7553 into main Apr 15, 2024
    1 check passed
    @GuyKh GuyKh deleted the device-details branch April 15, 2024 14:21
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    None yet

    1 participant