Skip to content

fix: add agent exec abstraction #15717

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

Merged
merged 4 commits into from
Dec 4, 2024
Merged

fix: add agent exec abstraction #15717

merged 4 commits into from
Dec 4, 2024

Conversation

sreya
Copy link
Collaborator

@sreya sreya commented Dec 2, 2024

This PR adds an agentexec.Execer to circumvent a bug I introduced that surfaces when trying to test agent subpackages. There were some obvious code smells with reading environment variables past startup but I thought agentexec was contained enough that I could get away with it...unfortunately enabling it in dogfood and then also trying to run the agent/* suite of tests results in a ton of problems.

I'm not really sure if there's a more ergonomic way to enable this API but I'm all ears if someone has a better alternative.

Copy link
Member

@deansheather deansheather left a comment

Choose a reason for hiding this comment

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

Looks good, I actually like this pattern more honestly

Copy link
Member

@mafredri mafredri left a comment

Choose a reason for hiding this comment

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

Clean implementation, nice job 👍🏻

@sreya sreya changed the title fix: pass through agent exec abstration fix: add agent exec abstration Dec 4, 2024
@sreya sreya changed the title fix: add agent exec abstration fix: add agent exec abstraction Dec 4, 2024
@sreya sreya merged commit ce573b9 into main Dec 4, 2024
33 checks passed
@sreya sreya deleted the jon/agentexecer branch December 4, 2024 21:30
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