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

refactor(typespec): Use typespec generated models #766

Merged
merged 9 commits into from
Oct 29, 2024

Conversation

creatorrr
Copy link
Contributor

@creatorrr creatorrr commented Oct 29, 2024

  • feat(typespec): Add granular integration definitions
  • feat(agents-api): Update integration defs
  • feat(integrations): Remove unused integrations
  • feat(typespec): Add provider cards
  • fix(typespec): Remove redundant wikipedia setup
  • refactor(integrations): Use typespec generated models

Important

Refactor integration models to use typespec-generated models, add granular integration definitions, update agents API, and configure CI for linting and testing.

  • Behavior:
    • Refactor execute_integration() in execute_integration.py to use BaseIntegrationDef instead of IntegrationDef.
    • Remove unused integrations like browserbase and hacker_news.
  • Models:
    • Add BaseIntegrationDef and specific integration models like BraveIntegrationDef, EmailIntegrationDef, etc.
    • Add provider cards for integrations like BraveProviderCard, EmailProviderCard, etc.
  • CI/CD:
    • Add lint-integrations-service-pr.yml and test-integrations-service-pr.yml for CI linting and testing.
  • Misc:
    • Update pyproject.toml to use datamodel-code-generator version 0.26.2.
    • Add __main__.py to integrations-service for entry point.
    • Remove redundant setup in wikipedia integration.

This description was created by Ellipsis for 191b319. It will automatically update as commits are pushed.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot 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 to me! Reviewed everything up to a2b1a4f in 1 minute and 20 seconds

More details
  • Looked at 7979 lines of code in 67 files
  • Skipped 2 files when reviewing.
  • Skipped posting 3 drafted comments based on config settings.
1. agents-api/agents_api/activities/execute_integration.py:7
  • Draft comment:
    The type of the integration parameter should be updated to BaseIntegrationDef in the function signature to reflect the change in the function body.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable:
    The comment is accurate as it points out a change that was made in the diff. The type of the 'integration' parameter was updated in the function signature, which aligns with the comment's suggestion. This is a clear and actionable comment related to the code change.
    The comment might be seen as stating the obvious since the change is already made in the diff. However, it does confirm that the change aligns with the intended update.
    While the change is already made, the comment serves as a confirmation that the update was necessary and correctly implemented.
    The comment is correct and relevant to the change made in the diff. It should be kept as it confirms the update to the function signature.
2. agents-api/agents_api/autogen/Agents.py:28
  • Draft comment:
    Ensure that the pattern constraint for the name field does not conflict with the empty string default value.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The removal of the default value for the name field in the Agent class and similar classes is a good change as it aligns with the Pydantic best practices for default values. However, the pattern constraint should be checked to ensure it doesn't conflict with the empty string default.
3. agents-api/agents_api/workflows/task_execution/__init__.py:18
  • Draft comment:
    The type of the integration parameter should be updated to BaseIntegrationDef in the function signature to reflect the change in the function body.
  • Reason this comment was not posted:
    Marked as duplicate.

Workflow ID: wflow_NknjuUiEgzVIWybr


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot 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 to me! Incremental review on 191b319 in 43 seconds

More details
  • Looked at 122 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. .github/workflows/lint-integrations-service-pr.yml:57
  • Draft comment:
    Add a newline at the end of the file for POSIX compliance.
  • Reason this comment was not posted:
    Confidence changes required: 10%
    The GitHub Actions workflows for linting and testing are well-structured, but they both lack a newline at the end of the file. This is a minor issue but considered a best practice for POSIX compliance.
2. .github/workflows/test-integrations-service-pr.yml:50
  • Draft comment:
    Add a newline at the end of the file for POSIX compliance.
  • Reason this comment was not posted:
    Confidence changes required: 10%
    The GitHub Actions workflows for linting and testing are well-structured, but they both lack a newline at the end of the file. This is a minor issue but considered a best practice for POSIX compliance.

Workflow ID: wflow_aEjJIeaWW3GlMNCe


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@HamadaSalhab HamadaSalhab merged commit 226013a into dev Oct 29, 2024
4 checks passed
@HamadaSalhab HamadaSalhab deleted the r/typespec-integrations branch October 29, 2024 13:18
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.

2 participants