Skip to content

⚡ Use context manager for safe file handling in search #36

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

Open
savvides wants to merge 1 commit into
base: main
Choose a base branch
from
Open

⚡ Use context manager for safe file handling in search #36

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
27 changes: 14 additions & 13 deletions bin/idstack-learnings-search
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,20 +49,21 @@ matches = []
for src in sources:
is_global = 'global' in src
try:
for line in open(src):
try:
d = json.loads(line)
if type_filter and d.get('type') != type_filter:
continue
if keyword:
insight = (d.get('insight', '') + ' ' + d.get('key', '')).lower()
if keyword not in insight:
with open(src) as f:

@gemini-code-assist gemini-code-assist Bot Jun 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Explicitly specifying the encoding when opening files is highly recommended to ensure portability and correctness across different environments.\n\nIn environments with a non-UTF-8 default locale (such as minimal Docker containers, CI environments, or systems running under the C locale), open(src) will fall back to the system's default encoding (e.g., ASCII). If the learnings file contains any non-ASCII characters (like curly quotes, accented characters, or emojis), iterating over the file will raise a UnicodeDecodeError. Because of the outer except Exception: pass block, this exception will be silently caught, causing the script to stop reading the rest of the file without any warning. Specifying encoding='utf-8' explicitly prevents this issue.

        with open(src, encoding='utf-8') as f:

for line in f:
try:
d = json.loads(line)
if type_filter and d.get('type') != type_filter:
continue
if is_global:
d['_source'] = 'global'
matches.append(json.dumps(d))
except Exception:
pass
if keyword:
insight = (d.get('insight', '') + ' ' + d.get('key', '')).lower()

@gemini-code-assist gemini-code-assist Bot Jun 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Using d.get('insight', '') can still return None if the key 'insight' is explicitly set to null in the JSON line. If either 'insight' or 'key' is None, the string concatenation d.get('insight', '') + ' ' + d.get('key', '') will raise a TypeError. This exception will be caught by the inner except Exception: pass block, causing the matching line to be silently skipped.\n\nUsing d.get('insight') or '' ensures that if the value is None (or missing), it gracefully defaults to an empty string. An f-string makes this concatenation clean and readable.

                        insight = f\"{d.get('insight') or ''} {d.get('key') or ''}\".lower()

if keyword not in insight:
continue
if is_global:
d['_source'] = 'global'
matches.append(json.dumps(d))
except Exception:
pass
except Exception:
pass

Expand Down