Skip to content

Added cache to get_settings #1151

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.

Sign up for GitHub

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

Merged
jokob-sk merged 1 commit into from
Aug 31, 2025
+46 −1
Merged

Added cache to get_settings #1151

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 46 additions & 1 deletion server/helper.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@
# Register NetAlertX directories
INSTALL_PATH="/app"

LASTCACHEDATE=''
CACHE={}

#-------------------------------------------------------------------------------
# DateTime
#-------------------------------------------------------------------------------
Expand Down Expand Up @@ -290,13 +293,55 @@ def write_file(pPath, pText):
#-------------------------------------------------------------------------------
# Return whole setting touple
def get_setting(key):

global LASTCACHEDATE
global CACHE

settingsFile = apiPath + 'table_settings.json'

###################################################
# fix by IR
###################################################

fileModifiedTime = os.path.getmtime(settingsFile)
mylog('debug', ['[Import table_settings.json] checking table_settings.json file '])
mylog('debug', ['[Import table_settings.json] LASTCACHEDATE :', LASTCACHEDATE])
mylog('debug', ['[Import table_settings.json] fileModifiedTime :', fileModifiedTime])
if fileModifiedTime == LASTCACHEDATE and CACHE:
mylog('debug', ['[Import table_settings.json] using cached version of table_settings.json '])
try:
return CACHE[key]
except KeyError:
mylog('none', [f'[Settings] ⚠ ERROR - setting_missing - Setting not found for key: {key} in cached file {settingsFile}'])

LASTCACHEDATE = fileModifiedTime

try:
with open(settingsFile, 'r') as json_file:
data = json.load(json_file)
for item in data.get("data",[]):
# map setkey to item
CACHE[item["setKey"]] = item
except (FileNotFoundError, json.JSONDecodeError, ValueError) as e:
# Handle the case when the file is not found, JSON decoding fails, or data is not in the expected format
mylog('none', [f'[Settings] ⚠ ERROR - JSONDecodeError or FileNotFoundError for file {settingsFile}'])
return None

try:
return CACHE[key]
except KeyError:
mylog('none', [f'[Settings] ⚠ ERROR - setting_missing - Setting not found for key: {key} in file {settingsFile}'])
return None

Comment on lines +305 to +334
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

⚠️ Potential issue

Fix: handle missing file (getmtime), avoid unnecessary reloads, rebuild cache atomically, and log exception details

  • os.path.getmtime can raise FileNotFoundError before the try/except. This will crash the server.
  • When mtime is unchanged and CACHE exists but key is missing, you still re-read the file; just return None.
  • Update LASTCACHEDATE only after a successful load; otherwise you can pin a bad mtime and never refresh.
  • Rebuild CACHE from scratch to drop stale keys; also guard missing setKey.
  • Use the caught exception (e) in the log to satisfy Ruff F841.
@@
-    fileModifiedTime = os.path.getmtime(settingsFile)
-    mylog('debug', ['[Import table_settings.json] checking table_settings.json file '])
-    mylog('debug', ['[Import table_settings.json] LASTCACHEDATE     :', LASTCACHEDATE])
-    mylog('debug', ['[Import table_settings.json] fileModifiedTime         :', fileModifiedTime])
-    if fileModifiedTime == LASTCACHEDATE and CACHE:
-        mylog('debug', ['[Import table_settings.json] using cached version of table_settings.json '])
-        try:
-            return CACHE[key]
-        except KeyError:
-            mylog('none', [f'[Settings] ⚠ ERROR - setting_missing - Setting not found for key: {key} in cached file {settingsFile}'])  
-    
-    LASTCACHEDATE = fileModifiedTime
-    
-    try:
-        with open(settingsFile, 'r') as json_file:
-            data = json.load(json_file)
-            for item in data.get("data",[]):
-                # map setkey to item
-                CACHE[item["setKey"]] = item
-    except (FileNotFoundError, json.JSONDecodeError, ValueError) as e:
-        # Handle the case when the file is not found, JSON decoding fails, or data is not in the expected format
-        mylog('none', [f'[Settings] ⚠ ERROR - JSONDecodeError or FileNotFoundError for file {settingsFile}'])                
-        return None
-    
-    try:
-        return CACHE[key]
-    except KeyError:    
-        mylog('none', [f'[Settings] ⚠ ERROR - setting_missing - Setting not found for key: {key} in file {settingsFile}'])  
-        return None 
+    try:
+        fileModifiedTime = os.path.getmtime(settingsFile)
+    except FileNotFoundError as e:
+        mylog('none', [f'[Settings] ⚠ ERROR - File not found: {settingsFile}: {e}'])
+        return CACHE.get(key) if CACHE else None
+
+    mylog('debug', ['[Import table_settings.json] checking table_settings.json file '])
+    mylog('debug', ['[Import table_settings.json] LASTCACHEDATE     :', LASTCACHEDATE])
+    mylog('debug', ['[Import table_settings.json] fileModifiedTime         :', fileModifiedTime])
+
+    if fileModifiedTime == LASTCACHEDATE and CACHE:
+        # Serve from cache without re-reading the file
+        if key in CACHE:
+            mylog('debug', ['[Import table_settings.json] using cached version of table_settings.json '])
+            return CACHE[key]
+        mylog('none', [f'[Settings] ⚠ ERROR - setting_missing - Setting not found for key: {key} in cached file {settingsFile}'])
+        return None
+
+    try:
+        with open(settingsFile, 'r') as json_file:
+            data = json.load(json_file)
+            # Rebuild cache from scratch to avoid stale keys persisting
+            CACHE.clear()
+            for item in data.get("data", []):
+                set_key = item.get("setKey")
+                if set_key is None:
+                    mylog('minimal', [f'[Settings] ⚠ WARNING - Skipping item without setKey in {settingsFile}'])
+                    continue
+                CACHE[set_key] = item
+            # Update mtime only after successful reload
+            LASTCACHEDATE = fileModifiedTime
+    except (FileNotFoundError, json.JSONDecodeError, ValueError) as e:
+        mylog('none', [f'[Settings] ⚠ ERROR - JSONDecodeError or FileNotFoundError for file {settingsFile}: {e}'])
+        return None
+
+    # Lookup after (re)load
+    if key in CACHE:
+        return CACHE[key]
+    mylog('none', [f'[Settings] ⚠ ERROR - setting_missing - Setting not found for key: {key} in file {settingsFile}'])
+    return None
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
fileModifiedTime = os.path.getmtime(settingsFile)
mylog('debug', ['[Import table_settings.json] checking table_settings.json file '])
mylog('debug', ['[Import table_settings.json] LASTCACHEDATE :', LASTCACHEDATE])
mylog('debug', ['[Import table_settings.json] fileModifiedTime :', fileModifiedTime])
if fileModifiedTime == LASTCACHEDATE and CACHE:
mylog('debug', ['[Import table_settings.json] using cached version of table_settings.json '])
try:
return CACHE[key]
except KeyError:
mylog('none', [f'[Settings] ⚠ ERROR - setting_missing - Setting not found for key: {key} in cached file {settingsFile}'])
LASTCACHEDATE = fileModifiedTime
try:
with open(settingsFile, 'r') as json_file:
data = json.load(json_file)
for item in data.get("data",[]):
# map setkey to item
CACHE[item["setKey"]] = item
except (FileNotFoundError, json.JSONDecodeError, ValueError) as e:
# Handle the case when the file is not found, JSON decoding fails, or data is not in the expected format
mylog('none', [f'[Settings] ⚠ ERROR - JSONDecodeError or FileNotFoundError for file {settingsFile}'])
return None
try:
return CACHE[key]
except KeyError:
mylog('none', [f'[Settings] ⚠ ERROR - setting_missing - Setting not found for key: {key} in file {settingsFile}'])
return None
try:
fileModifiedTime = os.path.getmtime(settingsFile)
except FileNotFoundError as e:
mylog('none', [f'[Settings] ⚠ ERROR - File not found: {settingsFile}: {e}'])
return CACHE.get(key) if CACHE else None
mylog('debug', ['[Import table_settings.json] checking table_settings.json file '])
mylog('debug', ['[Import table_settings.json] LASTCACHEDATE :', LASTCACHEDATE])
mylog('debug', ['[Import table_settings.json] fileModifiedTime :', fileModifiedTime])
if fileModifiedTime == LASTCACHEDATE and CACHE:
# Serve from cache without re-reading the file
if key in CACHE:
mylog('debug', ['[Import table_settings.json] using cached version of table_settings.json '])
return CACHE[key]
mylog('none', [f'[Settings] ⚠ ERROR - setting_missing - Setting not found for key: {key} in cached file {settingsFile}'])
return None
try:
with open(settingsFile, 'r') as json_file:
data = json.load(json_file)
# Rebuild cache from scratch to avoid stale keys persisting
CACHE.clear()
for item in data.get("data", []):
set_key = item.get("setKey")
if set_key is None:
mylog('minimal', [f'[Settings] ⚠ WARNING - Skipping item without setKey in {settingsFile}'])
continue
CACHE[set_key] = item
# Update mtime only after successful reload
LASTCACHEDATE = fileModifiedTime
except (FileNotFoundError, json.JSONDecodeError, ValueError) as e:
mylog('none', [f'[Settings] ⚠ ERROR - JSONDecodeError or FileNotFoundError for file {settingsFile}: {e}'])
return None
# Lookup after (re)load
if key in CACHE:
return CACHE[key]
mylog('none', [f'[Settings] ⚠ ERROR - setting_missing - Setting not found for key: {key} in file {settingsFile}'])
return None
🧰 Tools
🪛 Ruff (0.12.2)

324-324: Local variable e is assigned to but never used

Remove assignment to unused variable e

(F841)

🤖 Prompt for AI Agents 
In server/helper.py around lines 305-334: handle FileNotFoundError from
os.path.getmtime by wrapping the getmtime call in its own try/except and logging
the caught exception; if mtime is unchanged and CACHE exists, immediately return
CACHE.get(key) (which will be None) instead of re-reading the file; only set
LASTCACHEDATE after a successful load; rebuild the cache atomically by
populating a new temporary dict and then assigning it to CACHE (skip entries
missing "setKey"), and when catching exceptions include the exception variable
(e) in the mylog message so the actual error details are logged.

Copy link
Contributor Author

@ingoratsdorf ingoratsdorf Aug 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sweet suggestions, applying and testing.
Although some logic in CodeRabbot would have broken the routine.

Copy link
Contributor

@coderabbitai coderabbitai bot Aug 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ingoratsdorf Thanks for the feedback! I'm glad the suggestions were helpful even if some adjustments were needed. You're absolutely right to adapt the logic to fit the actual requirements - you know the codebase and runtime behavior better than I do.

Feel free to ping me if you run into any specific issues while testing or if you'd like me to review the final implementation. Looking forward to seeing how the caching performance improvement works out!

🐰💭 curious about the performance gains

###################################################
# / fix by IR
###################################################

try:

with open(settingsFile, 'r') as json_file:

data = json.load(json_file)
CACHE=data.get("data",[])

for item in data.get("data",[]):
if item.get("setKey") == key:
Expand Down