leoncvlt / blinkist-scraper

📚 Python tool to download book summaries and audio from Blinkist.com, and generate some pretty output
190 stars 36 forks source link

Keep all requirements consistent and up to date across ALL package managers #37

Closed rocketinventor closed 3 years ago

rocketinventor commented 3 years ago

Updated: Old text has been italicized and crossed out.

The version number listed for lxml is not the newest one available (4.6.2 for Python 3.9). The "==" operator forces pip to use lxml version 4.5.2, even if that meant downgrading it. This causes errors for lxml if "Microsoft Visual C++ 14.0" is not installed (because it is needed for compiling).

Also, some packages like certifi always need to be up to date (for securities' sake). They can now be updated as desired, and will not have to be rolled back in order to install blinkist-scraper.

Furthermore, the package versions and requirements lists specified by pyproject.toml, poetry.lock, and requirements.txt are all different, creating fragmentation in regards to what is actually installed when following the instructions.

Since it doesn't seem like any of these packages require a specific version, I have changed all of them to minimum version numbers (instead of required ones) using the >= operator.

This pull request updates the requirements.txt to exactly match the requirements specified by pyproject.toml and adds in a new tool to keep them in sync. Also, it ditches the poetry.lock file which was causing conflict about what actually needs to be installed. All the requirement specifications can now be kept in loc-step (instead of different specifications in different files, as was the case before). Pip already knows how to install dependencies properly. Now it can do so without having to have its hands tied. No C++ compiler needed!*

*anymore

Closes #28

leoncvlt commented 3 years ago

The project uses poetry for dependencies management, requirements.txt is actually generated from the poetry settings: https://python-poetry.org/docs/cli/#export

I think it would make more sense to make these changes on the pyproject.toml and keep one source of truth for the dependencies version

rocketinventor commented 3 years ago

Well, I thought that it was an issue from dependabot, but I checked now, and I can see that it only updated the .lockfile.

If the requirements.txt file is indeed being managed by Poetry, then I am not sure how to fix this issue...

Poetry doesn't seem to give any options about how the requirements.txt file is generated. Lxml and certifi are not specified directly as a requirement, but they are sub-dependencies. The only dependencies listed on the pyproject.toml file are:

chromedriver-autoinstaller = "^0.2.2" colorama = "^0.4.3" EbookLib = "^0.17.1" requests = "^2.24.0" selenium = "^3.141.0" selenium-wire = "^2.1.0"

Therefore, I would be unsure of how to fix this issue with Poetry (other than to manually edit the file).

Please feel free to advise, or update any settings that might fix this.

rocketinventor commented 3 years ago

@leoncvlt I added in a script to parse the pyproject.toml file and made a commit with the new generated requirements.txt file.

Also, I told Git to ignore the .lock file, because that really is meant to stay on one person's computer, only. This will have the side-benefit that dependabot will not have to bother us every time a sub-dependency is updated.

All that needs to be done to update the dependencies is to run py pyproject_parse.py in the project folder before committing. A nice and clean requirements.txt file that matches the pyproject.toml file will be automatically generated.

These changes should accommodate your goal to "keep one source of truth for the dependencies version". I think that this pull-request can be merged now.

leoncvlt commented 3 years ago

Thanks @rocketinventor, this is a good way to go!