Skip errors when reading a file fails #231
Conversation
When reading a file fails, e.g. because the file is not valid utf-8, it should be skipped instead of aborting the whole indexing run.
| if chunk.chunk_id not in self.cache.data["chunks_already_analyzed"]: | ||
| chunks_to_process.append(chunk) | ||
| self.cache.data["chunks_not_yet_analyzed"].add(chunk.chunk_id) | ||
| except Exception as e: |
There was a problem hiding this comment.
this is a bit generic, I think it might be a bad idea to skip any error
There was a problem hiding this comment.
it's probably very annoying to crash the server for a repo with hundreds or thousands of files just because one or two files cannot be read, however most errors might be errors that apply to all files, or majority of files, in which case probably the best way is to crash the server and allow the user to create an issue on github to fix it
There was a problem hiding this comment.
I am not sure how to design it well, I am now thinking that maybe there could be a counter, and it skips the first 5 errors or so, but crashes on the 5th?
There was a problem hiding this comment.
Since we know the total number of files, maybe it's better to use a relative cut-off e.g. abort when more than 1% of all files fail. Or alternatively abort when more than x% percent of files processed so far are errorneous. This should nicely catch the case where something is fundamentally wrong and all files are failing.
In larger repos the probability that there are no "weird" files tends to be very small ;) It would be good if the server would be somewhat robust with regard to file-ingestion.
The pre-commit check complains about print(): Shall we just use logging.error() in the server or do you have something else in mind for log messages?
There was a problem hiding this comment.
yeah, making it a % makes sense to me!
The pre-commit check complains about print(): Shall we just use logging.error() in the server or do you have something else in mind for log messages?
Yeah, I think it would make sense to use logging.error()
kantord
force-pushed
the
main
branch
2 times, most recently
from
89aa53c
to
12b4145
Compare
When reading a file fails, e.g. because the file is not valid utf-8, it should be skipped instead of aborting the whole indexing run.
This addresses #227