-
-
Notifications
You must be signed in to change notification settings - Fork 588
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
fix: remove URLs in pydantic error messages #662
Conversation
There was a problem hiding this 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 5c2a359 in 32 seconds
More details
- Looked at
70
lines of code in3
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. instructor/__init__.py:26
- Draft comment:
Setting environment variables at runtime can lead to inconsistent behavior across different parts of the application, especially in a multi-threaded or asynchronous context. Consider setting this environment variable outside the application scope, for example through configuration management or directly in the environment where the application runs. - Reason this comment was not posted:
Confidence changes required:80%
The PR aims to remove URLs from Pydantic error messages by setting an environment variable. The implementation sets the environment variablePYDANTIC_ERRORS_INCLUDE_URL
to "0" in theset_env_variable
function inutils.py
. This function is called in__init__.py
to ensure the variable is set when the package is used. The testtest_pylance_url_config
intest_function_calls.py
checks if the URL is not included in the error message, which aligns with the PR's goal. However, setting environment variables at runtime can be problematic because it might not affect already imported modules or those imported in parallel at startup. This could lead to inconsistent behavior where some parts of the application still show URLs in error messages.
Workflow ID: wflow_iQ2aBd5qo3ROeUh0
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
I would prefer this to be a piece of the documentation rather than something that we force users to use. |
This does sound like a sane default to me, especially when retries come into the picture. Would be cumbersome to have to set this env variable each time. But maybe something like a config file makes more sense instead? |
3d90201
to
0b3e28e
Compare
I have been waiting for this fix since March-2024. I am with @ssonal I bet you will disable it every time to avoid the URL message in the outputs in your documentation. Example:
(just my opinion) Just document how to enable if someone ever wants to see it and move on. In any case, I do welcome the addition of |
see #757 |
Fixes #509
Pydantic includes error URLs in ValidationErrors which means extra tokens used in LLM calls.
Pydantic relies on an environment variable to control this setting and is on by default. https://github.com/pydantic/pydantic-core/blob/e1fc99dd3207157aad77defc20ab6873fd268b5b/python/pydantic_core/_pydantic_core.pyi#L818
This change sets the environment variable dynamically when the package is invoked.
Summary:
This PR sets an environment variable to remove URLs from Pydantic error messages and includes a test to verify the change.
Key points:
PYDANTIC_ERRORS_INCLUDE_URL
to0
inset_env_variable
in/instructor/utils.py
set_env_variable
in/instructor/__init__.py
/tests/test_function_calls.py
to verify URLs are not in Pydantic error messagesGenerated with ❤️ by ellipsis.dev