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

chore: improving client typing #1036

Merged

Conversation

BalanaguYashwanth
Copy link
Contributor

@BalanaguYashwanth BalanaguYashwanth commented Dec 13, 2024

Relates to:

#656

Risks

Medium

  • It could effect the twitter client agent authentication flow

Background

What does this PR do?

Configured the multiple character agents with multiple twitter accounts, to run in parallel

What kind of change is this?

Bug fixes

  • Multiple twitter clients works with multiple agents running simultaneously

Documentation changes needed?

Yes, twitter client related docs need changes, already added it

Testing

Where should a reviewer start?

  • When starting the agent with twitter client, it should post a tweet and start interacting with the tweet

Files to be reviewed:

  • agent/src/index.ts
  • review the all files under
    • packages/client-twitter/src
  • packages/core/src/environment.ts
  • packages/core/tests/environment.test.ts
  • packages/core/src/types.ts

Detailed testing steps

  • Configure the multiple characters with their respecitve twitter client config
  • Start the agent
  • Check if the agent posts a tweet and starts interacting with the tweet
  • Check if the multiple agents are running in parallel

@BalanaguYashwanth BalanaguYashwanth changed the title Fix: Twitter client for multi agent Fix: Multiple Twitter accounts for multiple agents Dec 13, 2024
@BalanaguYashwanth BalanaguYashwanth changed the base branch from main to develop December 13, 2024 12:52
@BalanaguYashwanth BalanaguYashwanth marked this pull request as ready for review December 13, 2024 12:53
@BalanaguYashwanth BalanaguYashwanth changed the title Fix: Multiple Twitter accounts for multiple agents Fix: Dedicated Twitter client for agents Dec 13, 2024
@BalanaguYashwanth BalanaguYashwanth changed the title Fix: Dedicated Twitter client for agents Fix: Dedicated Twitter client for different character agents Dec 13, 2024
@BalanaguYashwanth BalanaguYashwanth changed the title Fix: Dedicated Twitter client for different character agents Fix: Different Twitter accounts for different character agents Dec 13, 2024
@dev-cz
Copy link

dev-cz commented Dec 13, 2024

Yes, this is a good fix. Lets get this one merged. A gap on the Twitter client, when running multiple agents.

@odilitime
Copy link
Collaborator

Definitely some code improvements here but how does using config improve on just using a secret/settings in each runtime?

@BalanaguYashwanth
Copy link
Contributor Author

@odilitime If i want to configure each twitter account to the each agent, then using a secret/settings may not resolve the issue.

I configured in a way that if you didn't mentioned any config, it will take from secret/settings in each runtime or else it will take from config where custom credentials will loaded that helps you which account to load which specific client.

@BalanaguYashwanth
Copy link
Contributor Author

BalanaguYashwanth commented Dec 13, 2024

@odilitime got it, Is this you are expecting, I didn't looked it this way. Yeah i think this instruction i didn't checked in docs.

so i created new PR with config setup. I think you can discard my PR. If below code works

"name": "Eliza",
    "clients": ["twitter"],
    "plugins": [],
    "modelProvider": "openai",
    "settings": {
        "secrets": {
            "TWITTER_USERNAME": "XXXX",
            "TWITTER_PASSWORD": "XXXXX",
            "TWITTER_EMAIL": "XXXXXXX"
        },
        "voice": {
            "model": "en_US-hfc_female-medium"
        }
    },

@odilitime
Copy link
Collaborator

I like a lot of the work here, I may merge it and then take out the config stuff. You do excellent typescript work

@BalanaguYashwanth
Copy link
Contributor Author

BalanaguYashwanth commented Dec 13, 2024

Thanks a lot @odilitime for appreciation, So current action item is

  • To remove config stuff from wherever new config changes are appended, and keep the typescript part.

Is that true ?

@odilitime odilitime changed the title Fix: Different Twitter accounts for different character agents chore: improving client typing Dec 13, 2024
@odilitime
Copy link
Collaborator

Yes. Also I like IAgentConfig stuff so keep that

@BalanaguYashwanth
Copy link
Contributor Author

@odilitime If i remove config part, then i need to remove the IAgentConfig part as well, Because there is no use of it.

So whats your opinion ?

@odilitime
Copy link
Collaborator

hrm I guess you're right, we don't need it currently and when I have a use I know where to copy the pattern from

@BalanaguYashwanth
Copy link
Contributor Author

ok thanks, let me remove config and get the updated PR

@odilitime odilitime deleted the branch elizaOS:develop December 13, 2024 19:07
@odilitime odilitime closed this Dec 13, 2024
@odilitime odilitime reopened this Dec 13, 2024
@@ -185,11 +185,13 @@ export class TwitterPostClient {
} else {
elizaLogger.log("Action processing loop disabled by configuration");
}
generateNewTweetLoop();
Copy link
Contributor Author

@BalanaguYashwanth BalanaguYashwanth Dec 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this piece of code, without this tweets won't generate into twitter
generateNewTweetLoop();

Note - Validate this piece of code, required or not

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I caught that and PR'd that earlier too

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it

@BalanaguYashwanth
Copy link
Contributor Author

BalanaguYashwanth commented Dec 13, 2024

Updated the PR @odilitime,

Also I tried to pass the integration-test but getting issue of below error, even through i am using the node v23.3.0

WARN  Unsupported engine: wanted: {"node":"23.3.0"} (current: {"node":"v23.4.0","pnpm":"9.4.0"})

More details - https://github.com/ai16z/eliza/actions/runs/12322163072/job/34395124357?pr=1036

@odilitime
Copy link
Collaborator

Just has a conflict in pnpm-lock.yaml I can't resolve. See if you can fix that and we'll get this merged!

…into 656--fix-twitter-multi-agent-support
@BalanaguYashwanth
Copy link
Contributor Author

@odilitime PR updated

@odilitime
Copy link
Collaborator

argh, more conflicts, sorry but I can't merge until those are resolved too

…into 656--fix-twitter-multi-agent-support
@odilitime odilitime merged commit c64b95d into elizaOS:develop Dec 14, 2024
3 of 4 checks passed
@BalanaguYashwanth
Copy link
Contributor Author

Thanks a lot @odilitime

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

Successfully merging this pull request may close these issues.

3 participants