AutoGPT
AutoGPT copied to clipboard
Migrate to poetry
Background
Based on https://github.com/Torantulino/Auto-GPT/pull/367
Changes
Migrate to poetry for requirements management.
Documentation
README.md is updated to reflect the change.
Test Plan
$ docker build . -t autogpt-fork
$ docker run -it --rm autogpt-fork
PR Quality Checklist
- [x] My pull request is atomic and focuses on a single change.
- [x] I have thoroughly tested my changes with multiple different prompts.
- [x] I have considered potential risks and mitigations for my changes.
- [x] I have documented my changes clearly and comprehensively.
- [x] I have not snuck in any "extra" small tweaks changes
@nponeccop please have a look
You're fixing all the version numbers in poetry. Isn't half the point of poetry that it can auto-figure-out compatible version numbers?
@p-i- you're right. We can:
- Let poetry figure out the versions freely (PR updated with this). I can submit a PR to update the tests, if we make sure to run them in the CI pipeline we should be pretty safe. See https://github.com/Torantulino/Auto-GPT/discussions/1117 for potential problem discussion.
- ... or pin major versions
- ... or just hope any breaking changes will be handled at the time of change
Whats your take on this?
I will sync the fork once we agree on the version handling per above. At the rate this codebase is changing it will look totally different once this PR is approved for merge. =)
@p-i- @richbeales any chance of input on the version resolution question so I can rebase this PR onto the latest master?
@rickythefox My vote would be for letting Poetry figure the versions out, rather than pinning versions.
Yes it's possible that some version update could cause an issue, but it feels so much cleaner to keep it floating and fix in the unusual event of a conflict.
How about we stick with (1) for now?
@p-i- perfect, then my last update handled that II will rebase within a free hours
Updated and pushed. Note that I pinned playsound = "1.2.2"
as 1.3.0 does not seem to build and 1.2.2 was the version used in requirements.txt anyway. If you want I can also include deleting requirements.txt into this PR. @nponeccop @p-i-
@rickythefox 2 deps that are in master are missing from the poetry deps:
pymilvus
redis
Please can you add them
Conflicting files Dockerfile
Fixed and added pymilvus and redis. Dev dependencies are not installed when building docker image. @richbeales
@rickythefox thank you very much ! there was some issues on some pipelines plus recent dependencies commented so we kept your work commits and created https://github.com/Significant-Gravitas/Auto-GPT/pull/1736
Hey @rickythefox we started consolidating your pr into this PR. Then I started thinking about how it could affect regular users and this is my concern:
- this is a mainstream project, people might not know poetry, they might just have 2 hours of python code behind them (again, I am not sure about that)
- more things to type for users python -m autogpt becomes poetry run python -m autogpt.
- overhead
So I am not saying no. I am just saying I am not so sure about this being a good move for all users, including non developers / tinkerers / dev amateurs.
@merwanehamadi @BillSchumacher well, the way I see it poetry will be more user-friendly.
- Virtual environments are created by default. For someone not familiar with the venv, just using pip to install from requirements.txt would pollute the global python installation and likely lead to conflicts in package versions.
- Dependency resolution is nicely handled by poetry.
- Typing
poetry run
takes 10 keystrokes. As long as this is properly documented the problem is minimal. You can also usepoetry shell
and execute all subsequent commands from there. - I'm not sure you would be able to grasp the codebase with 2 hours of python behind you, poetry or no poetry (prose?). If you have previous experience with java/dotnet/js/clojure you would be familiar with tools like maven/nuget/npm/leiningen which would map nicely to poetry.
@merwanehamadi I'm coming to this issue trying to package this for a linux distribution. Packaging with poetry would make maintainers life much easier, ultimately if you want distribution, having this installable on any distribution with a simple command would be the best IMO. poetry will on top of that enable build reproducibility, in turn you could check binaries with a sha256 to verify noone tempered with it. I realise this is a minor benefit, but it reproducibility enables several linux distribution to actually package this. For example, it won't be possible to package this into nixos without poetry.
I am fine if we move forward with that, I love poetry, and I was the one pushing for it. As long as we have all users in mind, not just maintainers.
There aren't many open source projects like this that reached such a wide range of users persona
Resolved conflicts
@rickythefox CI is red
This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request.
@nponeccop should be ok now
Conflicts have been resolved! 🎉 A maintainer will review the pull request shortly.
This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request.
Conflicts have been resolved! 🎉 A maintainer will review the pull request shortly.
@nponeccop so will this be merged or should I close the PR?
I think yes, it will probably be merged, just not now. Plugins are in the works. It's a huge change so we would like to release them first. Probably tonight.
@nponeccop ok, I'll hold off merging any conflicts until you say go.
This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request.
This is a mass message from the AutoGPT core team. Our apologies for the ongoing delay in processing PRs. This is because we are re-architecting the AutoGPT core!
For more details (and for infor on joining our Discord), please refer to: https://github.com/Significant-Gravitas/Auto-GPT/wiki/Architecting
Related:
- #1179
- #1955
Any news on poetry adoption?