Address code review feedback
- Replace LIKE queries with exact matches in fallback lookups - Add comments clarifying fallback queries should not be needed - Fix duplicate comment in embed_metadata_into_pdf - Add missing file_id parameter to log_task_progress call Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>
This commit is contained in:
@@ -64,24 +64,23 @@ def embed_metadata_into_pdf(self, local_file_path: str, extracted_text: str, met
|
|||||||
logger.info(f"[{task_id}] Starting metadata embedding for: {local_file_path}")
|
logger.info(f"[{task_id}] Starting metadata embedding for: {local_file_path}")
|
||||||
log_task_progress(task_id, "embed_metadata_into_pdf", "in_progress", f"Embedding metadata into {os.path.basename(local_file_path)}", file_id=file_id)
|
log_task_progress(task_id, "embed_metadata_into_pdf", "in_progress", f"Embedding metadata into {os.path.basename(local_file_path)}", file_id=file_id)
|
||||||
|
|
||||||
# Get file_id from database if not provided
|
# Get file_id from database if not provided (fallback only, prefer passing file_id explicitly)
|
||||||
if file_id is None:
|
if file_id is None:
|
||||||
with SessionLocal() as db:
|
with SessionLocal() as db:
|
||||||
file_record = db.query(FileRecord).filter_by(local_filename=local_file_path).first()
|
file_record = db.query(FileRecord).filter_by(local_filename=local_file_path).first()
|
||||||
if file_record:
|
if file_record:
|
||||||
file_id = file_record.id
|
file_id = file_record.id
|
||||||
|
|
||||||
|
# Check for file existence; if not found, try the known shared tmp directory.
|
||||||
if not os.path.exists(local_file_path):
|
if not os.path.exists(local_file_path):
|
||||||
alt_path = os.path.join(settings.workdir, "tmp", os.path.basename(local_file_path))
|
alt_path = os.path.join(settings.workdir, "tmp", os.path.basename(local_file_path))
|
||||||
if os.path.exists(alt_path):
|
if os.path.exists(alt_path):
|
||||||
local_file_path = alt_path
|
local_file_path = alt_path
|
||||||
else:
|
else:
|
||||||
logger.error(f"[{task_id}] Local file {local_file_path} not found, cannot embed metadata.")
|
logger.error(f"[{task_id}] Local file {local_file_path} not found, cannot embed metadata.")
|
||||||
log_task_progress(task_id, "embed_metadata_into_pdf", "failure", "File not found")
|
log_task_progress(task_id, "embed_metadata_into_pdf", "failure", "File not found", file_id=file_id)
|
||||||
return {"error": "File not found"}
|
return {"error": "File not found"}
|
||||||
|
|
||||||
|
|
||||||
# Check for file existence; if not found, try the known shared tmp directory.
|
|
||||||
|
|
||||||
# Work on a safe copy in a secure temporary directory
|
# Work on a safe copy in a secure temporary directory
|
||||||
original_file = local_file_path
|
original_file = local_file_path
|
||||||
# Create a temporary file with the same extension as the original
|
# Create a temporary file with the same extension as the original
|
||||||
|
|||||||
@@ -26,12 +26,14 @@ def finalize_document_storage(self, original_file: str, processed_file: str, met
|
|||||||
logger.info(f"[{task_id}] Finalizing document storage for {processed_file}")
|
logger.info(f"[{task_id}] Finalizing document storage for {processed_file}")
|
||||||
log_task_progress(task_id, "finalize_document_storage", "in_progress", f"Finalizing: {os.path.basename(processed_file)}", file_id=file_id)
|
log_task_progress(task_id, "finalize_document_storage", "in_progress", f"Finalizing: {os.path.basename(processed_file)}", file_id=file_id)
|
||||||
|
|
||||||
# Get file_id from database if not provided
|
# Get file_id from database if not provided (fallback only, prefer passing file_id explicitly)
|
||||||
if file_id is None:
|
if file_id is None:
|
||||||
with SessionLocal() as db:
|
with SessionLocal() as db:
|
||||||
# Try to find by the original file path
|
# Only as a last resort, try to find by exact match on local_filename
|
||||||
|
# This should not be needed if file_id is passed correctly through the chain
|
||||||
|
tmp_path = os.path.join(settings.workdir, "tmp", os.path.basename(original_file))
|
||||||
file_record = db.query(FileRecord).filter(
|
file_record = db.query(FileRecord).filter(
|
||||||
FileRecord.local_filename.like(f"%{os.path.basename(original_file)}%")
|
FileRecord.local_filename == tmp_path
|
||||||
).first()
|
).first()
|
||||||
if file_record:
|
if file_record:
|
||||||
file_id = file_record.id
|
file_id = file_record.id
|
||||||
|
|||||||
@@ -128,11 +128,13 @@ def send_to_all_destinations(self, file_path: str, use_validator=True, file_id:
|
|||||||
logger.info(f"[{task_id}] Sending {file_path} to all configured destinations")
|
logger.info(f"[{task_id}] Sending {file_path} to all configured destinations")
|
||||||
log_task_progress(task_id, "send_to_all_destinations", "in_progress", f"Distributing: {os.path.basename(file_path)}", file_id=file_id)
|
log_task_progress(task_id, "send_to_all_destinations", "in_progress", f"Distributing: {os.path.basename(file_path)}", file_id=file_id)
|
||||||
|
|
||||||
# Get file_id from database if not provided
|
# Get file_id from database if not provided (fallback only, prefer passing file_id explicitly)
|
||||||
if file_id is None:
|
if file_id is None:
|
||||||
with SessionLocal() as db:
|
with SessionLocal() as db:
|
||||||
|
# Only as a last resort, try to find by basename match
|
||||||
|
# This should not be needed if file_id is passed correctly through the chain
|
||||||
file_record = db.query(FileRecord).filter(
|
file_record = db.query(FileRecord).filter(
|
||||||
FileRecord.local_filename.like(f"%{os.path.basename(file_path)}%")
|
FileRecord.local_filename == os.path.join(settings.workdir, "tmp", os.path.basename(file_path))
|
||||||
).first()
|
).first()
|
||||||
if file_record:
|
if file_record:
|
||||||
file_id = file_record.id
|
file_id = file_record.id
|
||||||
|
|||||||
Reference in New Issue
Block a user