Generate JSON files to be consumed by Vue.js UI #219
Conversation
There was a problem hiding this comment.
Thank you.
Main changes required are around the usage of type hints. I know mostly no type hints are used so far in current codebase, but we would like to have as many as possible. So please try at least to add as many as reasonably possible in the code you add or modify.
Second concern is that I don't know how to test this, do we have at least a test channel or set of playlists where we could check how this code behaves? If not, we should probably build one so that we avoid reinventing the test scenario every time we test this. It should be small enough so that we can scrape it quite quickly, but big enough to exercise most scenarii of the scraper.
Finally, I'm a bit ashamed to continue to create code with absolutely no tests. I don't know if you have any suggestion regarding how we could automate testing this at least a bit. Maybe an issue for later, scraper has worked without tests for year, we can maybe continue like this for few more years ^^
|
@benoit74 I've included a zimdump of a ZIM file I generated using this new code here. Can you check the JSON files and let me know if everything is ok? (I removed the video files since there is a limit of 25MB for things I can upload here) |
|
Additionally, you can try running the scraper on the new testing channel created in #221 with the following command: |
|
I added a new Edit 1 - Added |
There was a problem hiding this comment.
Some more things to fix (I'm still very pleased by the PR, I just dug a bit more and found more stuff ^^):
- see inline comments
- remove what we now know for sure is obsolete code and files:
make_html_filesfunction (and maybe others I've missed)- the whole
templatesfolder and files
- translation of the UI was pretty minimalist, most probably partially broken, now it is completely broken by this PR (and previous one) ; I suggest to remove all stuff linked to the previous translation since it is just useless/missleading now (
localesubfolder + installation oflocales-allin Dockerfile) and open a new issue "Create structure for new Youtube UI translations" (I can open the issue with details if you want)
There was a problem hiding this comment.
Two more small comments regarding the CHANGELOG.
Please take them into account and you are now good to go to rewrite the Git history of your branch to keep only a handful of meaningful commits, as discussed yesterday. I don't really mind about the exact number, but I don't want to keep stuff which is only linked to the back-and-forth exchanges we had on this PR and do not really make much sense for the history (long term goal of commit history is to understand why changes have been done, not how). Force push will be needed after the rewrite obviously. Do not hesitate to ask for help if what I expect is too blurry for you or if you face issues.
Ask for a new review once this has been done and I will merge this.
CHANGELOG
Outdated
| @@ -14,6 +14,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 | |||
| - Initialize new Vue.js project in `zimui` subfolder | |||
| - Update dependencies in pyproject.toml (pydantic, pyhumps, python-slugify) | |||
| - Update scraper to generate JSON files for `zimui` (#212) | |||
| - Remove template files (home.html, article.html) and `make_html_files` method in scraper.py | |||
There was a problem hiding this comment.
Please be more specific with something like Remove old UI files and methods: template files (home.html, article.html) and make_html_files method in scraper.py
There was a problem hiding this comment.
Changed in 9981785
CHANGELOG
Outdated
| @@ -14,6 +14,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 | |||
| - Initialize new Vue.js project in `zimui` subfolder | |||
| - Update dependencies in pyproject.toml (pydantic, pyhumps, python-slugify) | |||
| - Update scraper to generate JSON files for `zimui` (#212) | |||
| - Remove template files (home.html, article.html) and `make_html_files` method in scraper.py | |||
| - Remove locale folder and files | |||
There was a problem hiding this comment.
Please be more specific with something like Remove broken locale folder and files used for translation ; translation will be restored with #222
There was a problem hiding this comment.
Changed in 0d7050f
This PR adds the functionality for generating JSON files to be consumed by Vue.js UI as mentioned in #212. The JSON files being generated are:
channel.json: Contains information about the channelplaylists.json: Contains a list of all playlists in the channelplaylistsfolder for each playlist : Contains info about the playlist and the videos it contains. (This includes the "Uploads" of the channel as well)videosfolder for each video : Contains info about the specific videoThe JSON files are added "on the fly" to the ZIM as mentioned in #209. However the video files and branding files are still being copied from the build_dir.
Closes #212
Closes #220