Skip to content
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

Support loading YTDL_OPTIONS from file #311

Merged
merged 5 commits into from
Aug 19, 2023
Merged

Support loading YTDL_OPTIONS from file #311

merged 5 commits into from
Aug 19, 2023

Conversation

arabcoders
Copy link
Contributor

@arabcoders arabcoders commented Aug 13, 2023

This PR implements a way to load yt-dlp custom options from file using the same environment variable name, i also updated the README.md to reflect the changes.

This PR closes #210

app/main.py Outdated
@@ -12,6 +12,8 @@
from ytdl import DownloadQueueNotifier, DownloadQueue

log = logging.getLogger('main')
if __name__ == '__main__':
Copy link
Owner

Choose a reason for hiding this comment

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

Why has this moved here? There's no reason to move it...

Copy link
Contributor Author

@arabcoders arabcoders Aug 19, 2023

Choose a reason for hiding this comment

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

Because the call to Config() comes before the logging level is set as such if you use debug level it wont show up. at least from my testing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alexta69 I can move it back but the logging in the Config class will have to be changed to INFO or removed as it's won't show up otherwise.

@alexta69 alexta69 merged commit 0adb85b into alexta69:master Aug 19, 2023
@alexta69
Copy link
Owner

@arabcoders I reworked the PR a bit -- I was a bit concerned about trying to interpret the same env variable in two different ways (JSON and file path) so I switched to a separate env variable for specifying the file. I hope it works well for you. Thanks!

@arabcoders
Copy link
Contributor Author

@arabcoders I reworked the PR a bit -- I was a bit concerned about trying to interpret the same env variable in two different ways (JSON and file path) so I switched to a separate env variable for specifying the file. I hope it works well for you. Thanks!

Oh no worries thanks for merging the support for it. If you have a bit of time could you please take look at #310 i posted screenshot of an edge case that happens when download fail.

Thank You.

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.

Support loading youtubeDL options from a file
2 participants