add cache clearing and testing #10
Conversation
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #10 +/- ##
==========================================
+ Coverage 93.75% 93.95% +0.20%
==========================================
Files 11 12 +1
Lines 320 331 +11
==========================================
+ Hits 300 311 +11
Misses 20 20 ☔ View full report in Codecov by Sentry. |
|
FYI @lpicci96 |
|
Thanks @mharoruiz From a user perspective, they would import the The clear cache should apply to all the sdr functions and called as |
|
Good point! @lpicci96 Let me know what you think now. |
There was a problem hiding this comment.
Thanks @mharoruiz
I think we can simplify the functionality. From a user perspective, it doesn't matter if there is cached data or not. It is only important they know that after calling the function, there is indeed no cached data. Unless you have a strong argument, I would suggest simplifying the function by removing the counter, simplifying the log message, and clearing the cache regardless if there is cached data or not
src/imf_reader/sdr/clear_cache.py
Outdated
|
|
||
| if cleared_caches == 0: | ||
|
|
||
| logger.info("Unable to clear cache - No cached data") |
There was a problem hiding this comment.
This might not be a useful message for the user who doesn't necessarily need to know if there is/isn't cached data
src/imf_reader/sdr/clear_cache.py
Outdated
| or get_latest_date.cache_info().currsize > 0 | ||
| ): | ||
| get_holdings_and_allocations_data.cache_clear() | ||
| get_latest_date.cache_clear() |
There was a problem hiding this comment.
cache_clear should function normally even if there is no cached data, so the condition check and counter might be redundant
src/imf_reader/sdr/clear_cache.py
Outdated
|
|
||
| if cleared_caches == 0: | ||
|
|
||
| logger.info("Unable to clear cache - No cached data") |
There was a problem hiding this comment.
This may be misleading to a user as they might think there is an issue with the function call
| @@ -54,7 +54,10 @@ def format_date(month: int, year: int) -> str: | |||
|
|
|||
|
|
|||
| @lru_cache | |||
| def get_holdings_and_allocations_data(year: int, month: int): | |||
| def get_holdings_and_allocations_data( | |||
| month: int, | |||
There was a problem hiding this comment.
@mharoruiz you accidentally switched the order of these parameters which are causing an issue further down the pipeline. The functionfetch_allocations_holdings fails as a result. The tests are passing though. Please can you check this bug?
The cache clearing functionality looks good though. Once the issue is addressed I can merge
There was a problem hiding this comment.
Sorry about that! I though it would usability but forgot to adjust the references down the line. It should be fixed now.
Added cache clearing functionality to the following SDR modules:
Also updated tests accordingly.