Open prakash-2002-ramanathan opened 1 month ago
@omeryusufyagci, The file path in python is relative to the python file. But since the MediaHandler is outside the backend directory. We are going to the previous directory where the Mediahandler is present. This seemed like a simple fix
Hi @prakash-2002-ramanathan, I wanted to check with you how you're progressing.
Although I appreciate the effort directly on the core, #34 mainly addresses the backend, where additions like the response handler will be ready in place, pending an update of the core for full integration.
You can still tackle the update of the core, but I'd propose to do it on a separate PR, and focus here fully on the backend improvements. The backend should function the same as it does right now, while the necessary handlers implemented and ready for the integration.
Tests are a big part of this PR, and would be quite impactful as we get ready to make larger changes.
Please let me know if this sounds good to you. Thanks!
Hi @omeryusufyagci Yeah, that sounds good. I have already implemented changes in the core. Is that fine, just over 50 lines. I have written communication handlers for the core, but not used them I have written communication handlers for the BE, and have used them. Except for the communication with the core
Hi @prakash-2002-ramanathan, that's great to hear!
I would still like you to submit your work on the core as a separate PR though. Let's evaluate the work you've done on the backend, finalize, merge and then take a look at your work on the core separately. This is also to expedite the addition of backend tests in preparation of our first release.
Finally, could you please target this PR to the dedicated branch opened under #34? Thanks!
Looking forward to see the changes!
Hi @omeryusufyagci , have stashed all the changes made in the core, only the changes in backend have been pushed.
The CLI nature of the core and backend communication is currently preserved.
Fixed the imports and removed the old app.py
You can run the new app by going to the backend directory and then running app.py.
Hi @omeryusufyagci , fixed all your requested changes. Please have a look
Hey @omeryusufyagci , could you explain how do you want this
download_media
function to be tested. Is it ok for you if I mock the yt_dlp.YoutubeDL()
function? I tried to download the youtube video and check it with a previously downloaded video. But sometimes the byte by byte comparison causes problem., since the content on youtube is dynamic and changes everytime when it's downloaded
Hey @omeryusufyagci , could you explain how do you want this
download_media
function to be tested. Is it ok for you if I mock theyt_dlp.YoutubeDL()
function? I tried to download the youtube video and check it with a previously downloaded video. But sometimes the byte by byte comparison causes problem., since the content on youtube is dynamic and changes everytime when it's downloaded
Hey @prakash-2002-ramanathan, you could try to test via the extracted metadata instead of the video itself to test if yt-dlp works fine. If we can get the metadata, we should likely be OK. You could, however, mock it as well. Byte-by-byte is not the best, I know... I'll open an issue about that. Thanks!
Hello @omeryusufyagci, I have addressed all your requested changes. Please have a look
@prakash-2002-ramanathan please note the update on the branch you're targeting that was required to add urgent functionalities. Please ensure the added functionality is kept in this PR. Thank you!
Hello @omeryusufyagci
This PR addresses #34. Please see if the current work is satisfactory
Possible issues:
config.json
config_file_path
in MediaHandlermain
to pass theconfig.json
file directory.I have not removed the original app.py. Not updated the README. Have not updated the requiremets.pip, some minor work is pending.