Fix zero-sized files (and some warnings)#252
Merged
Merged
Conversation
redrun45
changed the title
Collaborator
Author
|
Found a new failure mode! Best not merge until that's addressed. I'll post again, or remove and re-submit the PR, once it is. |
Fixes LibriVox#119, by removing the Validator code that would fetch MP3 files from remote servers. This was useful before Uploader became prevalent, but we no longer need the feature now.
This data, when returned by this endpoint, is never used in the js client[1]. It also causes server-side warnings! A little bit of progress for LibriVox#153. [1]: https://github.com/LibriVox/librivox-catalog/blob/master/public_html/js/private/validator/index.js#L29
redrun45
changed the title
Collaborator
Author
|
This little fix is ready now. Had a variable scoping issue before, which I can now see after circling back to it. 🐑 Priority on this one is low - not at all pressing. Something for me to do while we wait for the other bot-shoe to drop. Apart from that, I do hope to have some other improvements headed this way relatively soon. |
notartom
approved these changes
May 22, 2025
notartom
left a comment
notartom
left a comment
Member
There was a problem hiding this comment.
I love me anything that removes code :)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The first fixes #119, by removing the Validator code that would fetch MP3 files from remote servers. A bad fetch (from a typo) would often result in a zero-sized file in the Validator folder.
This 'fetch' feature was handy before Uploader became prevalent, but is no longer used. Better to remove than to fix. Admin discussion:
https://forum.librivox.org/viewtopic.php?p=2409190#p2409190
The next fixes #31 on dev, where mere warnings still cause a hang. In prod, the only cause I see for hanging is those zero-sized files we just addressed. We can consider this fixed as well, until we hear otherwise.
Future-musings:
This does look ripe for a re-factor (now
_local_fileand_copy_filecan be merged, for example), but I'll leave that be for now. I wouldn't mind removing thetest_*functions either (they don't look to automate well), but we might as well keep them in sync while they remain.