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

Add component decorator to AzureOpenAIGenerator #7694

Closed
wants to merge 3 commits into from

Conversation

GalRabin
Copy link

Related Issues

Proposed Changes:

The wrapped doesn't triggered on inheritance therefor it doesn't added to the registry, therefor I added it as well to the super class as well

How did you test it?

manual verification on my code

Notes for the reviewer

Checklist

@GalRabin GalRabin requested a review from a team as a code owner May 13, 2024 19:38
@GalRabin GalRabin requested review from davidsbatista and removed request for a team May 13, 2024 19:38
@CLAassistant
Copy link

CLAassistant commented May 13, 2024

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ davidsbatista
❌ GalRabin
You have signed the CLA already but the status is still pending? Let us recheck it.

@vblagoje vblagoje added the ignore-for-release-notes PRs with this flag won't be included in the release notes. label May 13, 2024
@vblagoje
Copy link
Member

vblagoje commented May 13, 2024

Very nice @GalRabin , thank you for this contribution. Normally we require unit tests for contribution (if applicable) but in this case we'll be adding a suite of tests just for component in the pipelines (de)serialization. Perhaps in this case we can make an exception as these tests are going to get added soon. cc @anakin87 @davidsbatista

@vblagoje vblagoje added this to the 2.1.2 milestone May 13, 2024
@vblagoje
Copy link
Member

@GalRabin there seem to be some minor failures, related to import order in the files changed. Please have a look, should be a simple fix to satisfy this validator

@coveralls
Copy link
Collaborator

coveralls commented May 13, 2024

Pull Request Test Coverage Report for Build 9076921259

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 3 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.002%) to 90.41%

Files with Coverage Reduction New Missed Lines %
components/generators/azure.py 3 78.95%
Totals Coverage Status
Change from base Build 9074873557: 0.002%
Covered Lines: 6533
Relevant Lines: 7226

💛 - Coveralls

@anakin87
Copy link
Member

  • Can you please sign the CLA?
  • Can you please also add the component decorator to the AzureOpenAIChatGenerator?

@davidsbatista
Copy link
Contributor

@GalRabin, thanks for contributing 👍🏽

@vblagoje @anakin87 I'm also OK with merging as-is, and then we add the missing tests.

@davidsbatista
Copy link
Contributor

@GalRabin sorry to close it, we needed to merge it and move fast, and we couldn't do it without you having the CLA signed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ignore-for-release-notes PRs with this flag won't be included in the release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deserialize - AzureOpenAIGenerator
6 participants