Skip to content

fix(label): raise CSVError instead of sys.exit on duplicate identifier #65

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
gkorland wants to merge 3 commits into
base: master
Choose a base branch
from
Open

fix(label): raise CSVError instead of sys.exit on duplicate identifier #65

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1bf0dbd
fix(label): raise CSVError instead of sys.exit on duplicate identifier
gkorland Apr 26, 2026
709ae57
fix(bulk_insert): catch CSVError from process_entities and exit cleanly
invalid-email-address Apr 29, 2026
eb19db3
fix(merge): resolve conflict in bulk_insert.py - combine CSVError han…
Copilot Apr 29, 2026
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
12 changes: 8 additions & 4 deletions falkordb_bulk_loader/bulk_insert.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from falkordb import FalkorDB

from .config import Config
from .exceptions import CSVError
from .label import Label
from .query_buffer import QueryBuffer
from .relation_type import RelationType
Expand Down Expand Up @@ -256,10 +257,13 @@ def bulk_insert(
)
logger.debug(f"Parsed {len(reltypes)} relation CSV file(s).")

logger.debug("Processing nodes...")
process_entities(labels)
logger.debug("Processing relations...")
process_entities(reltypes)
try:
logger.debug("Processing nodes...")
process_entities(labels)
logger.debug("Processing relations...")
process_entities(reltypes)
except CSVError as e:
sys.exit(str(e))
Comment on lines +260 to +266

@coderabbitai coderabbitai Bot Apr 29, 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.

⚠️ Potential issue | 🟠 Major

CSVError handling scope is too narrow for consistent CLI behavior.

Line 260 only wraps process_entities(...). parse_schemas(...) (Lines 252-257) can also raise CSVError (e.g., relation/header validation), which would still escape uncaught in CliRunner(catch_exceptions=False) scenarios.

Suggested fix 
-    # Read the header rows of each input CSV and save its schema.
-    logger.debug("Parsing node CSV schemas...")
-    labels = parse_schemas(Label, query_buf, nodes, nodes_with_label, config)
-    logger.debug(f"Parsed {len(labels)} node CSV file(s).")
-    logger.debug("Parsing relation CSV schemas...")
-    reltypes = parse_schemas(
-        RelationType, query_buf, relations, relations_with_type, config
-    )
-    logger.debug(f"Parsed {len(reltypes)} relation CSV file(s).")
-
-    try:
+    try:
+        # Read the header rows of each input CSV and save its schema.
+        logger.debug("Parsing node CSV schemas...")
+        labels = parse_schemas(Label, query_buf, nodes, nodes_with_label, config)
+        logger.debug(f"Parsed {len(labels)} node CSV file(s).")
+        logger.debug("Parsing relation CSV schemas...")
+        reltypes = parse_schemas(
+            RelationType, query_buf, relations, relations_with_type, config
+        )
+        logger.debug(f"Parsed {len(reltypes)} relation CSV file(s).")
+
         logger.debug("Processing nodes...")
         process_entities(labels)
         logger.debug("Processing relations...")
         process_entities(reltypes)
     except CSVError as e:
         sys.exit(str(e))
📝 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
try:
logger.debug("Processing nodes...")
process_entities(labels)
logger.debug("Processing relations...")
process_entities(reltypes)
except CSVError as e:
sys.exit(str(e))
try:
# Read the header rows of each input CSV and save its schema.
logger.debug("Parsing node CSV schemas...")
labels = parse_schemas(Label, query_buf, nodes, nodes_with_label, config)
logger.debug(f"Parsed {len(labels)} node CSV file(s).")
logger.debug("Parsing relation CSV schemas...")
reltypes = parse_schemas(
RelationType, query_buf, relations, relations_with_type, config
)
logger.debug(f"Parsed {len(reltypes)} relation CSV file(s).")
logger.debug("Processing nodes...")
process_entities(labels)
logger.debug("Processing relations...")
process_entities(reltypes)
except CSVError as e:
sys.exit(str(e))
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@falkordb_bulk_loader/bulk_insert.py` around lines 260 - 266, The CSVError
except block is too narrow—expand the try/except to include the
parse_schemas(...) call so any CSVError raised during schema parsing or during
process_entities(labels)/process_entities(reltypes) is caught; specifically wrap
the call to parse_schemas(...) along with the logger.debug("Processing
nodes...")/process_entities(labels) and logger.debug("Processing
relations...")/process_entities(reltypes) in the same try and keep the existing
except CSVError as e: sys.exit(str(e)) behavior.


# Send all remaining tokens to Redis
logger.debug("Flushing remaining buffered data to FalkorDB...")
Expand Down
7 changes: 5 additions & 2 deletions falkordb_bulk_loader/label.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import click

from .entity_file import EntityFile, Type
from .exceptions import SchemaError
from .exceptions import CSVError, SchemaError

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -60,7 +60,10 @@ def update_node_dictionary(self, identifier):
)
sys.stderr.flush()
if self.config.skip_invalid_nodes is False:
sys.exit(1)
raise CSVError(
"Duplicate node identifier '%s' at %s:%d"
% (identifier, self.infile.name, self.reader.line_num)
)
self.query_buffer.nodes[identifier] = self.query_buffer.top_node_id
self.query_buffer.top_node_id += 1

Expand Down
Loading