Skip to content

Commit 7a5955f

Browse files
committed
review comments
1 parent fd9ef21 commit 7a5955f

File tree

4 files changed

+67
-158
lines changed

4 files changed

+67
-158
lines changed

src/odoo_data_flow/import_threaded.py

Lines changed: 61 additions & 155 deletions
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,46 @@ def _create_batches(
348348
yield i, batch_data
349349

350350

351+
def _get_model_fields(model: Any) -> Optional[dict]:
352+
"""Safely retrieves the fields metadata from an Odoo model.
353+
354+
This handles cases where `_fields` can be a dictionary or a callable
355+
method, which can vary between Odoo versions or customizations.
356+
357+
Args:
358+
model: The Odoo model object.
359+
360+
Returns:
361+
A dictionary of field metadata, or None if it cannot be retrieved.
362+
"""
363+
if not hasattr(model, "_fields"):
364+
return None
365+
366+
model_fields_attr = model._fields
367+
model_fields = None
368+
369+
if callable(model_fields_attr):
370+
try:
371+
# It's a method, call it to get the fields
372+
model_fields_result = model_fields_attr()
373+
# Only use the result if it's a dictionary/mapping
374+
if isinstance(model_fields_result, dict):
375+
model_fields = model_fields_result
376+
except Exception:
377+
# If calling fails, fall back to None
378+
log.warning("Could not retrieve model fields by calling _fields method.")
379+
model_fields = None
380+
elif isinstance(model_fields_attr, dict):
381+
# It's a property/dictionary, use it directly
382+
model_fields = model_fields_attr
383+
else:
384+
log.warning(
385+
f"Model `_fields` attribute is of unexpected type: {type(model_fields_attr)}"
386+
)
387+
388+
return model_fields
389+
390+
351391
class RPCThreadImport(RpcThread):
352392
"""A specialized RpcThread for handling data import and write tasks."""
353393

@@ -602,6 +642,7 @@ def _create_batch_individually(
602642
error_summary = "Fell back to create"
603643
header_len = len(batch_header)
604644
ignore_set = set(ignore_list)
645+
model_fields = _get_model_fields(model)
605646

606647
for i, line in enumerate(batch_lines):
607648
try:
@@ -626,34 +667,15 @@ def _create_batch_individually(
626667
# Apply safe field value conversion to prevent type errors
627668
safe_vals = {}
628669
for field_name, field_value in vals.items():
629-
# Get field type information if available
630670
clean_field_name = field_name.split("/")[0]
631-
field_type = None
632-
# Check if model has _fields attribute and get field metadata properly
633-
model_fields = None
634-
if hasattr(model, "_fields"):
635-
model_fields_attr = model._fields
636-
if callable(model_fields_attr):
637-
# It's a method, call it to get the fields
638-
model_fields = model_fields_attr()
639-
else:
640-
# It's a property/dictionary, use it directly
641-
model_fields = (
642-
model_fields_attr
643-
if (
644-
hasattr(model_fields_attr, "__iter__")
645-
and not callable(model_fields_attr)
646-
)
647-
else None
648-
)
649-
671+
field_type = "unknown"
650672
if model_fields and clean_field_name in model_fields:
651673
field_info = model_fields[clean_field_name]
652-
field_type = field_info.get("type")
674+
field_type = field_info.get("type", "unknown")
653675

654676
# Apply safe conversion based on field type
655677
safe_vals[field_name] = _safe_convert_field_value(
656-
field_name, field_value, field_type or "unknown"
678+
field_name, field_value, field_type
657679
)
658680

659681
clean_vals = {
@@ -879,153 +901,37 @@ def _execute_load_batch( # noqa: C901
879901
if len(load_lines[0]) > 10:
880902
log.debug(f"Full first load_line: {load_lines[0]}")
881903

882-
# EXTRA DEBUGGING: Check for problematic values in integer fields
883-
# Look for float string values like "1.0" in fields that should be integers
884-
for i, field_name in enumerate(load_header):
885-
clean_field_name = field_name.split("/")[
886-
0
887-
] # Handle external ID fields like 'parent_id/id'
888-
# Check if we have field metadata and this is an integer field
889-
# Check if model has _fields attribute and get field metadata properly
890-
model_fields = None
891-
if hasattr(model, "_fields"):
892-
model_fields_attr = model._fields
893-
# Check if it's callable first, but be careful about the result
894-
if callable(model_fields_attr):
895-
try:
896-
# It's a method, call it to get the fields
897-
model_fields = model_fields_attr()
898-
except Exception:
899-
# If calling fails, treat it as a dictionary anyway
900-
model_fields = (
901-
model_fields_attr
902-
if (
903-
hasattr(model_fields_attr, "__iter__")
904-
and not callable(model_fields_attr)
905-
)
906-
else None
907-
)
908-
else:
909-
# It's a property/dictionary, use it directly
910-
model_fields = (
911-
model_fields_attr
912-
if (
913-
hasattr(model_fields_attr, "__iter__")
914-
and not callable(model_fields_attr)
915-
)
916-
else None
917-
)
918-
919-
if model_fields and clean_field_name in model_fields:
920-
field_info = model_fields[clean_field_name]
921-
field_type = field_info.get("type")
922-
# Ensure it's iterable for the 'in' check later
923-
if not hasattr(model_fields, "__iter__") or callable(model_fields):
924-
# If it's not iterable or it's a callable, set to None
925-
model_fields = None
926-
if field_type in ("integer", "positive", "negative"):
927-
# Check first few rows for float-like values in integer fields
928-
for j, row in enumerate(load_lines[:5]): # Check first 5 rows
929-
if i < len(row):
930-
value = row[i]
931-
str_value = str(value) if value is not None else ""
932-
if (
933-
"." in str_value
934-
and str_value.replace(".", "")
935-
.replace("-", "")
936-
.isdigit()
937-
):
938-
log.warning(
939-
f"Potentially problematic float value '{str_value}' "
940-
f"found in integer field '{field_name}' (row {j + 1}). "
941-
f"This might cause 'tuple index out of range' errors."
942-
)
943-
try:
944-
log.debug(f"Attempting `load` for chunk of batch {batch_number}...")
945-
946904
# PRE-PROCESSING: Clean up field values to prevent type errors
947-
# Convert float string values like "1.0" to integers for integer fields
948905
# This prevents "tuple index out of range" errors in Odoo server processing
949-
processed_load_lines = []
950-
if hasattr(model, "_fields"):
906+
model_fields = _get_model_fields(model)
907+
if model_fields:
908+
processed_load_lines = []
951909
for row in load_lines:
952910
processed_row = []
953911
for i, value in enumerate(row):
954912
if i < len(load_header):
955-
field_name = load_header[i].split("/")[
956-
0
957-
] # Handle external ID fields like 'parent_id/id'
958-
# Check if model has _fields attribute and get field metadata properly
959-
model_fields = None
960-
if hasattr(model, "_fields"):
961-
model_fields_attr = model._fields
962-
# Check if it's callable first
963-
if callable(model_fields_attr):
964-
try:
965-
# It's a method, call it to get the fields
966-
model_fields_result = model_fields_attr()
967-
# Only use the result if it's a dictionary/mapping
968-
if isinstance(model_fields_result, dict):
969-
model_fields = model_fields_result
970-
else:
971-
model_fields = None
972-
except Exception:
973-
# If calling fails, fall back to None
974-
model_fields = None
975-
else:
976-
# It's a property/dictionary, use it directly
977-
# But only if it's actually a dictionary
978-
if isinstance(model_fields_attr, dict):
979-
model_fields = model_fields_attr
980-
else:
981-
model_fields = None
982-
983-
# Only process if we have valid field metadata and the field exists
984-
if model_fields and field_name in model_fields:
985-
field_info = model_fields[field_name]
986-
field_type = field_info.get("type")
987-
# Only convert for integer fields
988-
if field_type in ("integer", "positive", "negative"):
989-
str_value = str(value) if value is not None else ""
990-
# Convert float string values like "1.0", "2.0" to integers
991-
if "." in str_value:
992-
try:
993-
float_val = float(str_value)
994-
if float_val.is_integer():
995-
# It's a whole number like 1.0, 2.0 - convert to int
996-
processed_row.append(int(float_val))
997-
else:
998-
# It's a non-integer float like 1.5 - keep original to let Odoo handle
999-
processed_row.append(value)
1000-
except ValueError:
1001-
# Not a valid float - keep original to let Odoo handle
1002-
processed_row.append(value)
1003-
elif str_value.lstrip("-").isdigit():
1004-
# It's an integer string like "1", "-5" - convert to int
1005-
try:
1006-
processed_row.append(int(str_value))
1007-
except ValueError:
1008-
# Not a valid integer - keep original to let Odoo handle
1009-
processed_row.append(value)
1010-
else:
1011-
# Not a numeric string - keep original to let Odoo handle
1012-
processed_row.append(value)
1013-
else:
1014-
# For all other field types, keep original value
1015-
processed_row.append(value)
1016-
else:
1017-
# If field doesn't exist or model has no _fields, pass original value
1018-
processed_row.append(value)
913+
field_name = load_header[i]
914+
clean_field_name = field_name.split("/")[0]
915+
916+
field_type = "unknown"
917+
if clean_field_name in model_fields:
918+
field_info = model_fields[clean_field_name]
919+
field_type = field_info.get("type", "unknown")
920+
921+
converted_value = _safe_convert_field_value(
922+
field_name, value, field_type
923+
)
924+
processed_row.append(converted_value)
1019925
else:
1020-
# Safety check: if index doesn't match, keep original value
1021926
processed_row.append(value)
1022927
processed_load_lines.append(processed_row)
1023928
load_lines = processed_load_lines
1024929
else:
1025-
# If model has no _fields, use original values
1026930
log.debug(
1027931
"Model has no _fields attribute, using raw values for load method"
1028932
)
933+
try:
934+
log.debug(f"Attempting `load` for chunk of batch {batch_number}...")
1029935

1030936
res = model.load(load_header, load_lines, context=context)
1031937
if res.get("messages"):

src/odoo_data_flow/importer.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -294,7 +294,12 @@ def run_import( # noqa: C901
294294
log.warning("Attempting to read CSV with UTF-8 encoding explicitly...")
295295
# Note: polars doesn't expose encoding parameter directly in read_csv
296296
# The encoding issue should be handled at the file system level
297-
df = pl.read_csv(filename, separator=separator, encoding=encoding, truncate_ragged_lines=True)
297+
df = pl.read_csv(
298+
filename,
299+
separator=separator,
300+
encoding=encoding,
301+
truncate_ragged_lines=True,
302+
)
298303

299304
# Identify columns that end with /id suffix
300305
id_columns = [col for col in df.columns if col.endswith("/id")]

src/odoo_data_flow/lib/preflight.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
from typing import Any, Callable, Optional, Union, cast
99

1010
import polars as pl
11-
import tempfile
1211
from polars.exceptions import ColumnNotFoundError
1312
from rich.console import Console
1413
from rich.panel import Panel

src/odoo_data_flow/lib/relational_import.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -504,7 +504,6 @@ def _execute_write_tuple_updates(
504504
# Attempt to convert to float first to handle "123.0", then to int
505505
related_db_id_int = int(float(related_db_id))
506506
except (ValueError, TypeError):
507-
508507
raise ValueError(f"Invalid related_db_id format: {related_db_id}")
509508

510509
m2m_command = [(4, related_db_id_int, 0)] # Convert to proper tuple format

0 commit comments

Comments
 (0)