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

adding support for async validation #1318

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tvt173
Copy link

@tvt173 tvt173 commented Jan 25, 2025

Hi,

First of all, thanks for the great project.

While working with it, I eventually found a need to use an async validator for my response model. This PR adds support for async validation.

I noticed that there is some code in this repo regarding async validators (#856), which i tried using, but even when using the included async validator on my model, it never seems to be executed.

I am using https://github.com/team23/pydantic-async-validation, but the only requirement for this to work is that the response model has defined on it a model_async_validate method, and that the user is using an async client.


Important

Adds async validation support in process_response_async() by calling model_async_validate if present on response_model.

  • Behavior:
    • Adds async validation support in process_response_async() by checking for model_async_validate method on response_model and calling it if present.
    • This change is specific to async processing and does not affect synchronous processing.
  • Functions:
    • Modifies process_response_async() in process_response.py to include async validation step.

This description was created by Ellipsis for 76a7478. 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 76a7478 in 12 seconds

More details
  • Looked at 22 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. instructor/process_response.py:87
  • Draft comment:
    The async validation check if hasattr(response_model, "model_async_validate"): is repeated twice in the function. Consider refactoring to avoid redundancy.
  • Reason this comment was not posted:
    Confidence changes required: 80%
    The async validation is added correctly, but it is repeated in two places unnecessarily.
2. instructor/process_response.py:87
  • Draft comment:
    The check if hasattr(response_model, "model_async_validate"): and the subsequent call await response_model.model_async_validate(model) is repeated. Consider refactoring to avoid this repetition.
  • Reason this comment was not posted:
    Confidence changes required: 80%
    The code has repeated checks for hasattr(response_model, "model_async_validate") and the subsequent call to await response_model.model_async_validate(model). This violates the DRY principle.

Workflow ID: wflow_72xXgF8eJmcd0stN


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

@tvt173
Copy link
Author

tvt173 commented Feb 1, 2025

@jxnl are you happy with this PR? please let me know if it needs any modifications

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.

1 participant