👋 🤩 Convert to proper python module. Working Docker image. #329

dhensen · 2023-04-06T16:31:54Z

Convert to proper python module.

This also makes creating tests easier.

I adjusted README.md.

Bonus: Dockerfile fixed. I've tested it with a simple scenario (using custom search engine, pinecone, no speak, no azure) and it is running fine.

dhensen · 2023-04-06T16:33:23Z

@Torantulino this one would be really beneficial to your project as it makes the project have a real python module namespace to work with. This then makes tests discoverable in IDE's (vscode, etc).

In addidtion I fixed the Dockerfile and while at it made sure to have a full run inside of it to at least know that it works.

dhensen · 2023-04-06T17:02:14Z

the execute_python_file function needs some more work to function properly. We need some docker-in-docker work here. I can fix this later.

OannesGPT · 2023-04-06T18:14:45Z

Convert to proper python module.

This also makes creating tests easier.

I adjusted README.md.

Bonus: Dockerfile fixed. I've tested it with a simple scenario (using custom search engine, pinecone, no speak, no azure) and it is running fine.

When I try your updated version I do run in problems: When not specifying pinecone data it runs stuck in a connection error for it: raise MaxRetryError(_pool, url, error or ResponseError(cause))
urllib3.exceptions.MaxRetryError: HTTPSConnectionPool(host='controller.your-pinecone-region.pinecone.io', port=443): Max retries exceeded with url: /databases (Caused by NewConnectionError('<urllib3.connection.HTTPSConnection object at 0x0000022E899D4510>: Failed to establish a new connection: [Errno 11001] getaddrinfo failed'))

When running with pinecone data I do seem to run into problems with the free plan or am I doing something wrong? This is the final output error: "ThisWhen The index exceeds the project quota of 1 pods by 1 pods. Upgrade your account or change the project settings to increase the quota."

dhensen · 2023-04-06T19:15:08Z

Convert to proper python module.
This also makes creating tests easier.
I adjusted README.md.
Bonus: Dockerfile fixed. I've tested it with a simple scenario (using custom search engine, pinecone, no speak, no azure) and it is running fine.

When I try your updated version I do run in problems: When not specifying pinecone data it runs stuck in a connection error for it: raise MaxRetryError(_pool, url, error or ResponseError(cause)) urllib3.exceptions.MaxRetryError: HTTPSConnectionPool(host='controller.your-pinecone-region.pinecone.io', port=443): Max retries exceeded with url: /databases (Caused by NewConnectionError('<urllib3.connection.HTTPSConnection object at 0x0000022E899D4510>: Failed to establish a new connection: [Errno 11001] getaddrinfo failed'))

When running with pinecone data I do seem to run into problems with the free plan or am I doing something wrong? This is the final output error: "ThisWhen The index exceeds the project quota of 1 pods by 1 pods. Upgrade your account or change the project settings to increase the quota."

Are you running in docker or normally? I’m 99% sure my change has got nothing to do with this. Are you able to run successfully on master branch with pinewood credentials?

dhensen · 2023-04-06T19:17:05Z

Oh I see now… look at your logs. You probably have your-pinecone-region in your .env file, try filling in as much variables as possible or leave the value empty if you aren’t using the option, then retry

dhensen · 2023-04-07T20:38:36Z

Just rebased cleanly on latest master (c6d9022).

dhensen · 2023-04-07T20:39:29Z

@muneebable can you review again? there are no additional changes, just a rebase.

ryanpeach · 2023-04-08T03:33:00Z

@dhensen NOTICE: I merged this PR branch into my PR as they are practically the same #142

I recommend closing this PR and focusing on that PR

I will be merging other related PR's as well.

dhensen · 2023-04-08T10:30:33Z

@dhensen NOTICE: I merged this PR branch into my PR as they are practically the same #142

I recommend closing this PR and focusing on that PR

I will be merging other related PR's as well.

@ryanpeach
I like what you're doing in your PR, but there is still the issue with main.py vs main.py that needs to be resolved. In the main time I'm keeping my PR ready to be merged.

I also notice that there is not very much progress over the last few days on this repo. I hope @Torantulino will get some help / organizes this somehow before people are going to seriously have a contending fork.

ryanpeach · 2023-04-09T01:24:29Z

@dhensen I have resolved your concerns.

Yes the code quality in this repo concerns me in terms of maintainability. Without proper CICD, a module, and reliable tests, it's not maintainable in the slightest. Theres also a lot of use of globals, just not sure how much experience the creator has with python tbh. However, it is where the community is and I will stick with the community.

nponeccop · 2023-04-10T10:16:28Z

@dhensen Resolve the conflicts

dhensen · 2023-04-10T14:26:32Z

@nponeccop thanks for the ping! just solved conflicts.

@Torantulino If you could consider giving this some prio over other PR's even though there is a voting system. My argumentation:

this is not a feature, people will never vote for it because it does not give them anything exiting
however, being a proper module will make development life easier
this is as isolated as possible, but all other PR's always make conflicts, because import fixes are spread through the whole project.
Dockerfile fix is also included here because of the rename
This Dockerfile uses ENTRYPOINT instead of CMD, this makes is super easy to pass additional arguments just like when running without docker. Other PR docker files use CMD, this creates more noise/issues because now people will come up with other solutions/questions to pass arguments.
I accidentally slipped in a cloudflare 502 fix in this PR, I can remove it you prefer.

For what it's worth: I always run the bot to see if it is able to fully start. Willing to work on testsuite and CI/CD, but I see a some PR's already.

nponeccop · 2023-04-10T14:31:51Z

If you could consider giving this some prio over other PR

Well, I would like to get all minor PRs merged before. No major features such as new commands etc. Maybe we'll need a shim so that features can be incorporated and migrated gradually

dhensen · 2023-04-10T18:21:35Z

@nponeccop good to see your batching issue, great idea.

When switching from script-based importing to a proper python module it's difficult. I can't even think of what a shim for that would look like, any ideas? Ideally this one should be merged as fast as possible, because the longer we wait the harder it's going to be. In company projects I always put a feature freeze, then do this kind of thing, then unfreeze. But that's kind of impossible on a 20K stars trending github repo.

I'll just try to keep this one mergable

nponeccop · 2023-04-10T20:41:32Z

We already have a feature freeze. We don't merge major features (read: new commands) In my company I write a shim, but I don't even know Python so I cannot advice.

waynehamadi · 2023-04-11T14:49:52Z

@dhensen waiting for this to rebase this one:
#765

@Torantulino can we get this in ?

dhensen · 2023-04-12T07:46:09Z

Fixed conflicts.

(.venv) [dino@emilia Auto-GPT (autogpt-namespace-fix-imports ✗)]$  /usr/bin/env /home/dino/work/Auto-GPT/.venv/bin/python /home/dino/.vscode/extensions/ms-python.python-2023.6.
0/pythonFiles/lib/python/debugpy/adapter/../../debugpy/launcher 53871 -- -m autogpt --gpt3only --continuous 
Redis not installed. Skipping import.
Pinecone not installed. Skipping import.
Continuous Mode:  ENABLED
WARNING:  Continuous mode is not recommended. It is potentially dangerous and may cause your AI to run forever or carry out actions you would not usually authorise. Use at your own risk.
GPT3.5 Only Mode:  ENABLED
Welcome back!  Would you like me to return to being Entrepreneur-GPT?
Continue with the last settings?
Name:  Entrepreneur-GPT
Role:  an AI designed to autonomously develop and run businesses with the sole goal of increasing your net worth.
Goals: ['Increase net worth.', 'Develop and manage multiple businesses autonomously.', 'Play to your strengths as a Large Language Model.']
Continue (y/n): y
Using memory of type: LocalCache
ENTREPRENEUR-GPT THOUGHTS: I think I should start by reviewing my current net worth and evaluating my current businesses to determine their profitability.
REASONING: In order to increase my net worth, it's important to know my current worth and the profitability of my current businesses. This will allow me to make informed decisions about whether to continue investing in these businesses or to cut my losses and explore new opportunities.
PLAN: 
-  Calculate net worth
-  Evaluate current businesses
-  Examine profitability of each business
-  Determine whether to invest further or cut losses
CRITICISM: I should have already been keeping track of my net worth and regularly evaluating the profitability of my businesses. This will be a lesson to keep on top of those things in the future.
NEXT ACTION:  COMMAND = do_nothing ARGUMENTS = {}
SYSTEM:  Command do_nothing returned: No action performed.
- Thinking...

Quick run shows it still works.

dhensen · 2023-04-12T08:14:01Z

With e475474 my "502 catch slipped-in feature" disappeared. I'll be making an isolated PR for that one. So this PR is now isolated to converting in a module and fix the Dockerfile because of the rename from script to autogpt

nponeccop · 2023-04-12T08:15:15Z

README.md

@@ -187,7 +201,7 @@ Pinecone enables the storage of vast amounts of vector-based memory, allowing fo

 ### Setting up environment variables

-Simply set them in the `.env` file. 


Undo the whitespace fixes. We will fix them in a separate PR.

nponeccop · 2023-04-12T08:15:55Z

autogpt/json_parser.py

@@ -26,7 +26,7 @@
 """


-def fix_and_parse_json(    


A whitespace fix

oh damn.. how does this happen, my IDE or what?

nponeccop · 2023-04-12T08:16:10Z

autogpt/speak.py

@@ -73,7 +73,7 @@ def speak():
            success = eleven_labs_speech(text, voice_index)
            if not success:
                gtts_speech(text)
-


A whitespace fix

dhensen · 2023-04-12T08:25:12Z

Merging without adding whitespace is getting harder and harder. I hope this can be merged soon.

nponeccop · 2023-04-12T08:52:13Z

Yes yes, we working on fixing the whitespace once and for all. There is a whitespace PR that was supposed to get merged yesterday. And the changes are creeping in during auto-merge. Auto-merge is not perfect even if it doesn't detect conflicts. Also this infinite rebasing is because of a long PR backlog. We will try to improve, another person has volunteered yesterday to help.

dhensen · 2023-04-12T08:54:57Z

I have to start vanilla vim (zero plugins) to do merges in plaintext to also avoid whitespace being removed. No problem ofcourse. But I would like to know a time window for this merge, because small PR's keep coming in, at this rate this is never getting merged, and as the project grows, merging work on this branch continues to grow. ❤️

nponeccop · 2023-04-12T08:55:10Z

@dhensen There are conflicts again. You can fix them now, or wait until your issue is about to merge. Your PR is in the Batch 3, so you can wait for Batch 2 to get closed first.

But you should understand that fixing the conflicts sooner is easier, so it's up to you. I will only review the code without conflicts, due to lack of time.

waynehamadi · 2023-04-12T13:56:27Z

@dhensen @nponeccop I know we're going to get this in, I have faith in you 🙏
All we need is this whitespace fix to make @dhensen s life easier to rebase. I also have this issue in my IDE.
And thanks for your help @nponeccop to manage the PRs, I know we're asking a lot, we're just too excited about this project.

May I suggest one thing @dhensen ? squash all your commits into 1 and rewrite history (push force) for this PR.
In all your future pushes, just push -f into the same commit (using amend).
The downside of doing that is we will loose the history in this thread, but most of the commits you made after the main one are not important anyway.
I know you know that, but just in case: git rebase -i HEAD~50, then replace "pick" by squash in the 31 commits you made except one then push force

This will avoid painful rebase for others, they will just have one commit to rebase instead of 31. your changes can fit one commit called "create autogpt module", we don't need 30 commits here.

dhensen · 2023-04-12T17:45:03Z

@dhensen @nponeccop I know we're going to get this in, I have faith in you pray All we need is this whitespace fix to make @dhensen s life easier to rebase. I also have this issue in my IDE. And thanks for your help @nponeccop to manage the PRs, I know we're asking a lot, we're just too excited about this project.

May I suggest one thing @dhensen ? squash all your commits into 1 and rewrite history (push force) for this PR. In all your future pushes, just push -f into the same commit (using amend). The downside of doing that is we will loose the history in this thread, but most of the commits you made after the main one are not important anyway. I know you know that, but just in case: git rebase -i HEAD~50, then replace "pick" by squash in the 31 commits you made except one then push force

This will avoid painful rebase for others, they will just have one commit to rebase instead of 31. your changes can fit one commit called "create autogpt module", we don't need 30 commits here.

Thanks for the support @merwanehamadi, yes I thinking of doing that earlier today when I was looking at this branch' history earlier today.. what a complete mess.

nponeccop · 2023-04-12T17:50:28Z

Well, many people add merge commits instead of rebasing. Merging doesn't suffer from this work bloat the way rebasing does, but it has its own flaws. The next whitespace fix is #810. Hopefully tomorrow. Today we finally learned how to merge more quickly.

dhensen · 2023-04-12T18:22:24Z

I did it... rebasing was hell, halfway I aborted that and did the git reset --soft master trick. But while doing so already another PR got merged in 😢
When I'm done I'm gonna get a 🥇 for this PR 😆 😆

Onto the next rebase. Consider me a Jedi from now on, as I'll be using the force continuously on this PR now.

richbeales · 2023-04-14T16:05:45Z

autogpt/data/prompt.txt

@@ -0,0 +1,63 @@
+CONSTRAINTS:


/data/prompt.txt is no longer part of the project (now prompt.py)

Thanks Richard, I will remove it.

It has been removed

Also fixed the Dockerfile. Converting to module makes development easier. Fixes coverage script in CI and test imports.

waynehamadi · 2023-04-14T17:11:36Z

hey @dhensen we're close to the finish line. I think your PR is the likely candidate for the module change. All it needs is one last conflict fix

waynehamadi · 2023-04-14T18:22:42Z

I put your commit there plus added some imports and a redirect message: #1380

richbeales · 2023-04-14T19:08:23Z

closed, many thanks for your hard work on this

waynehamadi · 2023-04-14T19:14:18Z

thanks @dhensen

Co-authored-by: Luke <2609441+lc0rp@users.noreply.github.com>

dhensen changed the title ~~Convert to proper python module. Working Docker image.~~ 👋 🤩 Convert to proper python module. Working Docker image. Apr 6, 2023

muneebable previously approved these changes Apr 7, 2023

View reviewed changes

dhensen dismissed muneebable’s stale review via 9caaca3 April 7, 2023 20:37

dhensen force-pushed the autogpt-namespace-fix-imports branch from bef690b to 9caaca3 Compare April 7, 2023 20:37

ryanpeach mentioned this pull request Apr 8, 2023

Command registry #350

Closed

6 tasks

ryanpeach mentioned this pull request Apr 9, 2023

Renamed scripts folder to autogpt. Absolute Imports. isort. Fixed flake8 F401, E402 #142

Closed

nponeccop mentioned this pull request Apr 10, 2023

PRs batch 2 #673

Closed

1 task

nponeccop mentioned this pull request Apr 10, 2023

PR batch 3 #709

Closed

1 task

nponeccop suggested changes Apr 12, 2023

View reviewed changes

nponeccop previously approved these changes Apr 12, 2023

View reviewed changes

dhensen dismissed nponeccop’s stale review via 888cacb April 12, 2023 18:18

dhensen force-pushed the autogpt-namespace-fix-imports branch from 91bfd0c to 888cacb Compare April 12, 2023 18:18

dhensen force-pushed the autogpt-namespace-fix-imports branch 3 times, most recently from 8b42efc to 98205de Compare April 12, 2023 18:41

nponeccop previously approved these changes Apr 12, 2023

View reviewed changes

dhensen dismissed nponeccop’s stale review via ecf3f17 April 14, 2023 15:03

dhensen force-pushed the autogpt-namespace-fix-imports branch 3 times, most recently from 094cbd4 to 30dfe40 Compare April 14, 2023 15:07

nponeccop previously approved these changes Apr 14, 2023

View reviewed changes

richbeales reviewed Apr 14, 2023

View reviewed changes

Convert to python module named autogpt.

6c524af

Also fixed the Dockerfile. Converting to module makes development easier. Fixes coverage script in CI and test imports.

dhensen dismissed nponeccop’s stale review via 6c524af April 14, 2023 16:29

dhensen force-pushed the autogpt-namespace-fix-imports branch from 30dfe40 to 6c524af Compare April 14, 2023 16:29

nponeccop approved these changes Apr 14, 2023

View reviewed changes

waynehamadi mentioned this pull request Apr 14, 2023

Autogpt namespace fix imports #1380

Merged

5 tasks

richbeales closed this Apr 14, 2023

waynehamadi pushed a commit that referenced this pull request Sep 5, 2023

Improve TestRevenueRetrieval_1.1 task specification (#329)

d86ed40

Co-authored-by: Luke <2609441+lc0rp@users.noreply.github.com>

		@@ -187,7 +201,7 @@ Pinecone enables the storage of vast amounts of vector-based memory, allowing fo

		### Setting up environment variables

		Simply set them in the `.env` file.

👋 🤩 Convert to proper python module. Working Docker image. #329

👋 🤩 Convert to proper python module. Working Docker image. #329

Conversation

dhensen commented Apr 6, 2023

dhensen commented Apr 6, 2023

dhensen commented Apr 6, 2023

OannesGPT commented Apr 6, 2023

dhensen commented Apr 6, 2023

dhensen commented Apr 6, 2023 • edited Loading

dhensen commented Apr 7, 2023

dhensen commented Apr 7, 2023

ryanpeach commented Apr 8, 2023 • edited Loading

dhensen commented Apr 8, 2023

ryanpeach commented Apr 9, 2023

nponeccop commented Apr 10, 2023

dhensen commented Apr 10, 2023 • edited Loading

nponeccop commented Apr 10, 2023

dhensen commented Apr 10, 2023

nponeccop commented Apr 10, 2023

waynehamadi commented Apr 11, 2023

dhensen commented Apr 12, 2023

dhensen commented Apr 12, 2023 • edited Loading

nponeccop Apr 12, 2023

Choose a reason for hiding this comment

nponeccop Apr 12, 2023

Choose a reason for hiding this comment

dhensen Apr 12, 2023

Choose a reason for hiding this comment

nponeccop Apr 12, 2023

Choose a reason for hiding this comment

dhensen commented Apr 12, 2023

nponeccop commented Apr 12, 2023

dhensen commented Apr 12, 2023 • edited Loading

nponeccop commented Apr 12, 2023

waynehamadi commented Apr 12, 2023 • edited Loading

dhensen commented Apr 12, 2023

nponeccop commented Apr 12, 2023

dhensen commented Apr 12, 2023

richbeales Apr 14, 2023

Choose a reason for hiding this comment

dhensen Apr 14, 2023

Choose a reason for hiding this comment

dhensen Apr 14, 2023

Choose a reason for hiding this comment

waynehamadi commented Apr 14, 2023

waynehamadi commented Apr 14, 2023 • edited Loading

richbeales commented Apr 14, 2023

waynehamadi commented Apr 14, 2023

dhensen commented Apr 6, 2023 •

edited

Loading

ryanpeach commented Apr 8, 2023 •

edited

Loading

dhensen commented Apr 10, 2023 •

edited

Loading

dhensen commented Apr 12, 2023 •

edited

Loading

dhensen commented Apr 12, 2023 •

edited

Loading

waynehamadi commented Apr 12, 2023 •

edited

Loading

waynehamadi commented Apr 14, 2023 •

edited

Loading