Skip to content

Fix issue #353 : broken pdf doesn't fail job - #356

Open
Exino329 wants to merge 4 commits into
EPFLiGHT:mainfrom
Exino329:wip/ll-fail-job
Open

Exino329 wants to merge 4 commits into
EPFLiGHT:mainfrom
Exino329:wip/ll-fail-job

Conversation

@Exino329

Copy link
Copy Markdown

When an invalid PDF was inserted into the index-api, it called the PDF processor's process_batch method, which is an overload of the process_batch method from the Processor class. The method was catching the exception but not re-raising it, which caused the bug.

A regression test has also been added to tests/test_live_retriever_api.py.

The new behaviour is now:

image image

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

the failure of the job should be only in API mode, it should fail with just a logged warning in general. it would also be nice to make the job failure specify the exception

batch_results.append(result)
except Exception as e:
logging.error(f"Failed to process {file}: {str(e)}")
batch_results.append(None) # handle partial failures

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why remove this? it's a convenient way to save that there is no valid value for the failed file

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants