-
-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
change message on missing language data for video #227
base: main
Are you sure you want to change the base?
change message on missing language data for video #227
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #227 +/- ##
=====================================
Coverage 5.00% 5.00%
=====================================
Files 8 8
Lines 1098 1098
Branches 238 238
=====================================
Hits 55 55
Misses 1042 1042
Partials 1 1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking a bit more in-depth to this code, I believe that we were wrong on the issue analysis. Sorry for that.
I now don't get why the scraper keeps trying to find the language name for every video, while in fact these names should be fixed for the whole scrape.
I.e. once we know that en
is English
, we need to keep this information somewhere.
Also why are we searching for lang name for subtitles if they do not exists? I realize that I don't get your reasoning in fact.
Okay, um, I tried to drill down the code to understand the error and here's what I found (probably not 100% spot-on). When the scraper is called without the
The definition of a video being "translated" means metadata for the video, like title, etc has been written in that language and not necessarily that the audio is in that language. The logic for Then, there are also edge cases where a video exists in a language like English but it doesn't have any |
I''m sorry, I won't have time to delve into it seriously for the moment. Thank you for this analysis, and I will have to delve to be sure to implement the right thing. Keeping this PR open until then. |
Rationale
Sometimes, even though a video exists for a particular native language, there is no available subtitle/language data for the video in that language. This PR re-words the warning message to a more descriptive message. This resolves #216.
Changes
get
attribute is used as we are in an exception handler and using the[]
to access the canonical url might throw an error given this data is from an external API and we don't want to throw an exception from here.