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

ArgillaCallbackHandler to properly use default values for api_url and api_key #9113

Merged

Conversation

alvarobartt
Copy link
Contributor

Hi to @agola11 or whoever is reading this! 🤗

What's in this PR?

As of the recent PR at #9043, after some testing we've realised that the default values were not being used for api_key and api_url. Besides that, the default for api_key was set to argilla.apikey, but since the default values are intended for people using the Argilla Quickstart (easy to run and setup), the defaults should be instead owner.apikey if using Argilla 1.11.0 or higher, or admin.apikey if using a lower version of Argilla.

Additionally, we've removed the f-string replacements from the docstrings.

P.S. Regarding the Twitter/X mention feel free to do so at either https://twitter.com/argilla_io or https://twitter.com/alvarobartt and https://twitter.com/gabrielmbmb_, or both if applicable, otherwise, just the first Twitter/X handle.

alvarobartt and others added 3 commits August 11, 2023 10:23
Add semi-colon at the end of `push_to_argilla` since it returns the `FeedbackDataset` in Argilla since 1.14.0
Co-authored-by: Gabriel Martin <gabriel@argilla.io>
@vercel
Copy link

vercel bot commented Aug 11, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
langchain ⬜️ Ignored (Inspect) Visit Preview Aug 11, 2023 8:33am

@dosubot dosubot bot added Ɑ: models Related to LLMs or chat model modules 🤖:bug Related to a bug, vulnerability, unexpected error with an existing feature labels Aug 11, 2023
@baskaryan
Copy link
Collaborator

thanks @alvarobartt!

@baskaryan baskaryan merged commit f7ae183 into langchain-ai:master Aug 11, 2023
23 checks passed
@alvarobartt alvarobartt deleted the argilla-use-quickstart-defaults branch August 11, 2023 16:48
danielchalef pushed a commit to danielchalef/langchain that referenced this pull request Aug 11, 2023
… and `api_key` (langchain-ai#9113)

As of the recent PR at langchain-ai#9043, after some testing we've realised that the
default values were not being used for `api_key` and `api_url`. Besides
that, the default for `api_key` was set to `argilla.apikey`, but since
the default values are intended for people using the Argilla Quickstart
(easy to run and setup), the defaults should be instead `owner.apikey`
if using Argilla 1.11.0 or higher, or `admin.apikey` if using a lower
version of Argilla.

Additionally, we've removed the f-string replacements from the
docstrings.

---------

Co-authored-by: Gabriel Martin <gabriel@argilla.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤖:bug Related to a bug, vulnerability, unexpected error with an existing feature Ɑ: models Related to LLMs or chat model modules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants