feat: Add GetCities, GetCityStreets, GetOutagesByAddress APIs

[GuyKh]

What changes do you are proposing?

feat: Add GetCities, GetCityStreets, GetOutagesByAddress APIs

How did you test these changes?

manually

Closing issues

[GuyKh]

GetOutagesByAddress - requires a header: recaptchtoken
For now, not sure where to get it from...

[what-the-diff]

PR Summary

• Added New Endpoint Constants

  • New endpoints namely; GET_CITIES_URL, GET_CITY_STREETS_URL, GET_OUTAGES_BY_ADDRESS_URL have been added to const.py file. This helps in fetching specific information more efficiently.

• Improved Functionality for JWT Token

  • The function get_response_with_descriptor in the data.py file has been modified to handle cases where the jwt_token is None. This improves the handling of missing authentication tokens and makes the code more robust.

• New Functions in data.py

  • New functions have been added to improve data handling and retrieval. This includes functions designed to fetch lists of cities and specific cities by name, procure lists of streets for particular cities and specific streets by name, and retrieve outage information for a particular address. All these new functions will contribute to the enhancement of data interaction process.

• New Functions in iec_client.py

  • New functions in this file are meant for accessing and fetching various data including cities, streets and outage information. These new functionalities will make retrieving specific data much easier and faster.

• New Data Classes in address.py

  • The address.py module has been introduced where data classes for City and Street have been added, along with response decoders. This aids in organizing the addressing related data and its decoding more systematically.

• New Data Classes in outages.py

  • The new module outages.py is introduced holding data classes for dealing with outage information. Besides the data classes, a response decoder has also been added to interpret outage information responses. This change enables the application to provide a better handling of outage related information.

[GuyKh]

/review

[codiumai-pr-agent-pro]

PR Review

(Review updated until commit 1ec8003)

⏱️ Estimated effort to review [1-5]	3, because the PR introduces new APIs with caching and flexible input handling, which requires careful consideration of data flow, error handling, and integration with existing systems. The manual testing described seems thorough for initial validation, but automated tests would be beneficial for future maintenance and reliability.
🧪 Relevant tests	No
Insights from user answers	The developer implemented caching for cities and streets due to the static nature of this data, reducing unnecessary API calls. They also provided flexibility in function arguments to allow for both object and ID inputs, catering to different use cases. The manual testing process involved a specific flow, indicating a practical approach to validating the new features.
🔍 Possible issues	Possible Bug: The lack of null checks before extending the cities list in get_cities function could lead to potential errors if the API response is unexpectedly null or malformed.
	Performance Concern: Caching is implemented for cities and streets, but there's no mechanism described for cache invalidation or update, which could lead to stale data issues over time.
🔒 Security concerns	No

Code feedback:

---

relevant file	iec_api/data.py
suggestion	Consider implementing a mechanism for cache invalidation or refresh for the cities and city_streets caches to handle potential data updates over time. This could be a time-based invalidation or a manual refresh trigger. [important]
relevant line	cities: list[City] = []

---

relevant file	iec_api/data.py
suggestion	Add error handling for the API calls within get_cities and get_city_streets to gracefully handle cases where the API response is null or in an unexpected format. This could prevent potential runtime errors. [important]
relevant line	response = await _get_response_with_descriptor(session, None, GET_CITIES_URL, cities_decoder)

---

relevant file	iec_api/data.py
suggestion	For functions accepting both a model object and its ID as arguments (e.g., city: City | str | UUID), consider adding type checks and raising a clear error if the argument does not match expected types. This improves code robustness and readability. [medium]
relevant line	async def get_city_streets(session: ClientSession, city: City | str | UUID) -> Optional[list[Street]]:

---

relevant file	iec_api/data.py
suggestion	Optimize the conditional checks in get_city_streets by consolidating the isinstance checks for city into a single block to improve code readability and maintainability. [medium]
relevant line	if isinstance(city, str):

---

---

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

• With a configuration file, use the following template:

[pr_reviewer]
some_config1=...
some_config2=...

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

[GuyKh]

/reflect_and_review

[codiumai-pr-agent-pro]

Questions to better understand the PR:

1. I see that you have added caching for city and city streets. Is there a specific reason for this? Is the data not expected to change frequently?
2. In the get_outages function, you are accepting both the object (City or Street) and their IDs (str) as arguments. Is there a specific use case for this flexibility?
3. Can you elaborate on the testing process you followed for these changes? You mentioned manual testing, but could you provide more details?

Please respond to the questions above in the following format:

/answer

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

[GuyKh]

/answer

1. No, the data is rather static.
2. Yes, I want to allow the user to pass either City object (result of get_city for example) or just the UUID (to skip the city fetching if the user already has the ID in hand).
3. I tested this through the following flow:

        city = await client.get_city("חיפה")
        print(city)

        street = await client.get_city_street_by_name(city, "הרצל")
        print(street)

        outages = await client.get_outages_by_address(city, street, house_num="1")
        print(outages)

[codiumai-pr-agent-pro]

Persistent review updated to latest commit 1ec8003

[GuyKh]

/review -i

[codiumai-pr-agent-pro]

Incremental PR Review

⏮️ Review for commits since previous PR-Agent review Starting from commit 1ec8003.

⏱️ Estimated effort to review [1-5]	3, because the changes involve adding new functionalities and modifying existing ones, which requires a careful review of the logic and potential side effects. The use of caching mechanisms for cities and streets data introduces complexity that needs thorough testing.
🧪 Relevant tests	No
🔍 Possible issues	Thread Safety: The caching mechanism used for cities and city_streets may not be thread-safe. This could lead to inconsistent states in a multi-threaded environment.
	Memory Leak: Storing all cities and streets in memory could lead to a memory leak if the dataset is large or grows over time.
	Possible Bug: The check if len(cities) == 0: assumes that the cities list will only be populated once. If the underlying data changes, this cache will become stale.
🔒 Security concerns	No

Code feedback:

---

relevant file	iec_api/data.py
suggestion	Consider implementing a thread-safe caching mechanism or using a library designed for caching to avoid potential race conditions and ensure data consistency. [important]
relevant line	cities: list[City] = []

---

relevant file	iec_api/data.py
suggestion	To prevent a memory leak and manage the cache size, implement a cache eviction policy or limit the cache size. [important]
relevant line	city_streets: dict[str, list[Street]] = {}

---

relevant file	iec_api/data.py
suggestion	Use a more robust condition to check if the cities need to be re-fetched, such as timestamp-based invalidation, to handle data updates. [medium]
relevant line	if len(cities) == 0:

---

relevant file	iec_api/data.py
suggestion	Add error handling for the network requests within get_cities and get_city_streets to gracefully handle failures or unexpected responses. [important]
relevant line	response = await _get_response_with_descriptor(session, None, GET_CITIES_URL, cities_decoder)

---

---

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

• With a configuration file, use the following template:

[pr_reviewer]
some_config1=...
some_config2=...

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