argilla-io / argilla

Argilla is a collaboration platform for AI engineers and domain experts that require high-quality outputs, full data ownership, and overall efficiency.
https://docs.argilla.io/en/latest/
3.63k stars 339 forks source link

[DOCS] swap extra_headers for headers in updated sdk docs #5100

Closed burtenshaw closed 5 days ago

burtenshaw commented 1 week ago

This PR passes the extra headers pass to Argilla down to the http client so that argilla sdk can be used with authenticate deployment like provate HF spaces.

How Has This Been Tested

Checklist

burtenshaw commented 6 days ago

LFTM! One remark, are we testing this flow somewhere, otherwise we can update it?

No. I could unit test the client attribute.

review-notebook-app[bot] commented 6 days ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

burtenshaw commented 6 days ago

@davidberenstein1957 Thanks for pointing me to testing. On closer inspection I realised that the docs were wrong and we should be using headers not extra_headers.

@frascuchon can you confirm this please?

frascuchon commented 6 days ago

@davidberenstein1957 Thanks for pointing me to testing. On closer inspection I realised that the docs were wrong and we should be using headers not extra_headers.

@frascuchon can you confirm this please?

Yes, @burtenshaw. the Argilla client exposes the httpx.Cient init args in the extra kwargs. Let us add some docs to the Argilla.init describing this (as we do with the APIClient definition

frascuchon commented 6 days ago

We already have some unit tests checking this https://github.com/argilla-io/argilla/blob/fix/extra-headers/argilla/tests/unit/api/http/test_http_client.py