[FEATURE] update token retrieval logic

[not-lain]

Description

support token retrieval from colab secrets
this PR is inspired by huggingface_hub's get_token function
if token exists in secrets it will prompt the user if they want to access it or not

#Fixes : #5543

Type of change

• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Refactor (change restructuring the codebase without changing functionality)
• Improvement (change adding some improvement to an existing functionality)

How Has This Been Tested

Checklist

• I added relevant documentation
• I followed the style guidelines of this project
• I did a self-review of my code
• I made corresponding changes to the documentation
• I confirm My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• I have added relevant notes to the CHANGELOG.md file (See https://keepachangelog.com/)

[not-lain]

here's a minimalistic colab code to try this PR before merging:

!pip install -e "git+https://github.com/not-lain/argilla@update-token-retrieval#egg=argilla&subdirectory=argilla"

restart session then try this :

from argilla._api._token import get_token
get_token()

[not-lain]

cc @frascuchon for review

[burtenshaw]

Thanks for the PR. This looks really handy. I added some small comments. Two general things:

• On naming: Currently, we're referring to our server api credentials as 'credentials' and 'api_key', rather than 'token'. I think this code should use the same naming. There's been confusion between argilla api_key and hf token before.
• The logic at the moment is to get both or no values (key and url) from userdata and fall back to environ. Do you think there's a use case where the user just wants the key from the userdata?

[frascuchon]

Thanks @not-lain for the contribution. Yesterday I was thinking about this. Since argilla already has the huggingface_hub dependency, why not use the get_token function to configure the token? With this, we keep aligned with other HF packages. What do you think?

[burtenshaw]

@frascuchon If you want to try it, I used this colab: https://colab.research.google.com/drive/1Qh1DFo7rZD_qR-FqCXYSRyv4m2_Z8qIG?usp=sharing.

[not-lain]

thanks a lot for the review

On naming: Currently, we're referring to our server api credentials as 'credentials' and 'api_key', rather than 'token'. I think this code should use the same naming. There's been confusion between argilla api_key and hf token before.

thanks for letting me know, will fix this in the next commit.

The logic at the moment is to get both or no values (key and url) from userdata and fall back to environ. Do you think there's a use case where the user just wants the key from the userdata?

I can adapt my implementation to handle this case, I was a bit too skeptical because of the collab thread hanging forever issue (see huggingface/huggingface_hub#1952 ) and I wanted to make something that is robust instead of flexible.
Will try to make it work and commit to this pr once it's ready.

[burtenshaw]

I can adapt my implementation to handle this case, I was a bit too skeptical because of the collab thread hanging forever issue (see huggingface/huggingface_hub#1952 )

@not-lain Understood. I think it's a nice to have anyway, so if it compromises the functionality feel free to ignore the suggestion.

[burtenshaw]

@not-lain @frascuchon We should put this is 2.3.0!

[frascuchon]

Maybe we can generalize this function to retrieve a secret from colab, an use it multiple times, instead of hardcoding the expected variables:

_get_secret_from_google_colab("ARGILLA_AP_KEY")
_get_secret_from_google_colab("ARGILLA_API_URL")

We could do something similar to the get_token and the _get_token_from_environment functions:

ARGILLA_API_KEY = get_secret("ARGILLA_API_KEY")
ARGILLA_API_URL = get_secret("ARGILLA_API_URL")

[not-lain]

hi @frascuchon I have tried your changes which can be tested via the following code (different branch from this one)

!pip install -e "git+https://github.com/not-lain/argilla@test-colab-lock#egg=argilla&subdirectory=argilla"

which produced some weird behavior when running the following code

from argilla._api._token import get_secret
key = get_secret("ARGILLA_API_KEY")
url = get_secret("ARGILLA_API_URL")

initialize your secrets and turn them off

then click [ cancel, grant, cancel ]
as you might have noticed 3 tabs popped up which is quite strange and I might attribute this to the shared global lock

        global _IS_GOOGLE_COLAB_CHECKED

not entirely sure of this but this is what I have discovered so far and I wanted to report my findings

[not-lain]

Apologies I have been looking at this from a different angle, colab turns out to handle things a slightly bit differently fromthe the normal way I was looking at this.
after careful testing, it turns out that

from argilla._api._token import get_secret
ARGILLA_API_URL = get_secret("ARGILLA_API_URL") 
ARGILLA_API_KEY = get_secret("ARGILLA_API_KEY")

is DIFFERENT from

import argilla as rg
rg.client._api.APIClient() 

I have no clue how and why this happened, but I will report my findings to colab and let them figure this out.

I will now proceed to merge the changes that @frascuchon has recommended

[not-lain]

This PR should be ready for review, you can test this out by using the following code

1. install

!pip install -e "git+https://github.com/not-lain/argilla@update-token-retrieval#egg=argilla&subdirectory=argilla"

2. restart session
3. run the following code

import argilla as rg
rg.client._api.APIClient()

[frascuchon]

Thanks @not-lain for taking into account my comments, and apologize for the inconvenience. So, finally, did you manage the errors applying my changes? Does the env loading work as expected? The current code looks nice!

[not-lain]

Hi @frascuchon, yes I can indeed confirm that everything is working perfectly.

[frascuchon]

Thanks @not-lain !

[frascuchon]

@burtenshaw Does it make sense to add it somewhere in the docs?

[burtenshaw]

@burtenshaw Does it make sense to add it somewhere in the docs?

@frascuchon Yeah. I would just add a section to client API reference that mentions support for colab userdata. But let's merge this to develop, and I can open a docs PR.