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

Inconsistent project dependencies #132

Closed
3 tasks
croxxx opened this issue May 7, 2022 · 4 comments
Closed
3 tasks

Inconsistent project dependencies #132

croxxx opened this issue May 7, 2022 · 4 comments

Comments

@croxxx
Copy link

croxxx commented May 7, 2022

Observation

This is a small detail I found after digging around the code to analyze #130 and #131. It seems this is related to two recent merge requests #110 and #122.
Without being too familiar with Python tooling, I noticed that the dependency definitions differ between pyproject.toml, requirements.txt and requirements-dev.txt.

File Version
pyproject.toml anki = "2.1.49"
requirements.txt anki==2.1.40; python_version >= "3.8"
requirements-dev.txt anki==2.1.49; python_version >= "3.8"

My assumption would be for those to be identical, yet the requirement explicitly stated in #110 (comment) is violated by requirements.txt.

Suggestion

Assuming dependencies are to be exclusively managed by the tool reading pyproject.toml, I suggest the following:

  • Remove requirements.txt and requirements-dev.txt as they are auto-generated on build.
  • Add requirements.txt and requirements-dev.txt to .gitignore to prevent checking in inconsistent versions in the future.
  • Clearly update build documentation stating how dependencies are managed in this project.
@VikashKothary
Copy link
Member

VikashKothary commented May 16, 2022

Hi @croxxx,

Thanks for bringing this to my attention. The Pyproject.toml is the correct version and I will update the other files now to fix the inconsistency.

I understand your suggestion however poetry is still a relatively new tool in the scope of Python. While I love it, there is a large number of users who use other tools like virtualenv and conda. The generated requirements.txt files are kept there to allow those users to use their preferred package manager.

It's also important to note that generating the requirements.txt requires poetry which means that they would have to download it even if they don't want to use it.

There is a PR open which hopes to prevent this issue from occuring in the future by using GitHub Actions to automatically update the reqirements.txt files when the pyproject.toml is updated. See: #126.

Is this an acceptable solution?

@croxxx
Copy link
Author

croxxx commented May 17, 2022

Ah, that is a reasonable approach.

In addition to the changes proposed in the PR I can imagine some sort of sanity check downstream.
For example, the Docker build could always recreate requirements files based on the poetry base. That way even bad dependencies cannot "poison" the build and the specific versions are locked anyways. Downside of this approach would be masking errors, of course.

As the immediate issue has been resolved for now and there is changes proposed improving the general behavior, feel free to close this.

@ohdearaugustin
Copy link
Contributor

Should be fixed now with #126

@VikashKothary
Copy link
Member

The PR has been merged. So hopefully we should not have this issue in the future.

Thank you @croxxx for raising this issue and @ohdearaugustin for your help in resolving it. It is much appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants