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

feat: add runcontext + retryable + emitter to tool #429

Merged
merged 7 commits into from
Feb 28, 2025

Conversation

vabarbosa
Copy link
Contributor

Which issue(s) does this pull-request address?

Contributes-to: #362

Description

this PR adds the RunContext, Retryable and Emitter calls to Tool. Tool related examples were updated accordingly

Checklist

  • I have read the contributor guide
  • I have signed off on my commit
  • Linting passes: poe lint
  • Formatting is applied: poe format
  • Unit tests pass: poe test --type unit
  • E2E tests pass: poe test --type e2e
  • Tests are included
  • Documentation is changed or added
  • Commit messages and PR title follow conventional commits

Signed-off-by: va <va@us.ibm.com>
@vabarbosa vabarbosa added the python Python related functionality label Feb 28, 2025
@vabarbosa vabarbosa self-assigned this Feb 28, 2025
@vabarbosa vabarbosa requested a review from a team as a code owner February 28, 2025 15:53
Copy link
Contributor

@Tomas2D Tomas2D left a comment

Choose a reason for hiding this comment

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

I noticed that you switch to async which is good, but shouldn't we also change the signature of _run method to be async?

def __init__(self, options: dict[str, Any] | None = None) -> None:
super().__init__(options)
# replace any non-alphanumeric char with _
formatted_name = re.sub(r"\W+", "_", self.name).lower()
Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed that you were already using this helper somewhere, please create a utility function in utils/strings.py

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Signed-off-by: va <va@us.ibm.com>
Signed-off-by: va <va@us.ibm.com>
Signed-off-by: va <va@us.ibm.com>
Tomas2D
Tomas2D previously approved these changes Feb 28, 2025
@vabarbosa vabarbosa merged commit d300354 into main Feb 28, 2025
5 checks passed
@vabarbosa vabarbosa deleted the feat/tools-emitter branch February 28, 2025 17:48
@ajbozarth ajbozarth mentioned this pull request Feb 28, 2025
6 tasks
@vabarbosa vabarbosa mentioned this pull request Mar 3, 2025
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
python Python related functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants