feature/downloading-youtube-videos #169
Conversation
…placing the large video file
Codecov Report
@@ Coverage Diff @@
## main #169 +/- ##
==========================================
- Coverage 94.56% 94.54% -0.02%
==========================================
Files 50 50
Lines 2558 2587 +29
==========================================
+ Hits 2419 2446 +27
- Misses 139 141 +2
Continue to review full report at Codecov.
|
evamaxfield
changed the title
cdp_backend/utils/file_utils.py
Outdated
| dst: str | ||
| The location of the downloaded file. | ||
| """ | ||
| dst = Path(str(dst) + ".mp4") |
There was a problem hiding this comment.
Was about to merge but one last look I think catches this.
This will always overwrite the supplied dst correct?
I think there simply needs to be an if dst is not None before this statement.
Additionally the docstring / parameter typing should be updated to dst: Optional[Path] = None
There was a problem hiding this comment.
No, I assumed that dst is not empty since if dst is None, it would be a assigned a value, as seen here. I can throw an exception if dst is None though. WDYT?
There was a problem hiding this comment.
Ohhhh i see what you are doing. Yes. My mistake. Okay so then only nitpick would be instead of doing this str(dst) + ".mp4" string addition. Path has a function called with_suffix that is "safer" imo.
Path(str(dst)).with_suffix(".mp4")There was a problem hiding this comment.
But yes, you are correct. My mistake on on the overwrite vs simply adding the mp4
There was a problem hiding this comment.
I just pushed a new change based on your feedback.
Link to Relevant Issue
This pull request resolves #161
Description of Changes
Make it possible for the program to download a YouTube video embedded onto a website. This is needed for the Portland scraper.