-
Notifications
You must be signed in to change notification settings - Fork 406
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
feat: add login
function for storing Argilla credentials
#3582
Conversation
c35df01
to
82ad0d3
Compare
60051d2
to
03ea530
Compare
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## develop #3582 +/- ##
===========================================
+ Coverage 90.52% 90.53% +0.01%
===========================================
Files 256 257 +1
Lines 13577 13624 +47
===========================================
+ Hits 12290 12335 +45
- Misses 1287 1289 +2
☔ View full report in Codecov by Sentry. |
The URL of the deployed environment for this PR is https://argilla-quickstart-pr-3582-ki24f765kq-no.a.run.app |
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.
LGTM! I just left some minor comments and suggestions before merging 👍🏻
Co-authored-by: Alvaro Bartolome <alvaro@argilla.io>
for more information, see https://pre-commit.ci
74ca678
to
a95dd47
Compare
# Description **NOTE**: This PR is not ready yet, it assumes the [PR 3582](#3582) is already integrated. This PR adds a new cli command `python -m argilla workspaces list`. Closes #3587 **Type of change** (Please delete options that are not relevant. Remember to title the PR according to the type of change) - [x] New feature (non-breaking change which adds functionality) **How Has This Been Tested** No tests were done yet. Maybe a new module under `tests/unit/tasks` should be added? In that case, the current implementation (when properly logged), prints to the console a table like: ![image](https://github.com/argilla-io/argilla/assets/56895847/4ec57268-70d4-495c-9198-2a5692f9bc50) **Checklist** - [ ] I added relevant documentation - [x] I followed the style guidelines of this project - [x] I did a self-review of my code - [ ] I made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] I filled out [the contributor form](https://tally.so/r/n9XrxK) (see text above) - [ ] I have added relevant notes to the `CHANGELOG.md` file (See https://keepachangelog.com/) --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: gabrielmbmb <gmartinbdev@gmail.com>
…o#3582) # Description This PR includes a new function called `login` that will try to login to the Argilla server using the provided credentials. If it success, the provided credentials will be saved to `~/.cache/argilla/credentials.json` file. The `argilla.client.client.Argilla.__init__` method has been updated to try to use the stored credentials if the `api_url` and `api_key` arguments are not provided. Closes argilla-io#3581 **Type of change** - [x] New feature (non-breaking change which adds functionality) **How Has This Been Tested** I've added unit tests and tried in a local development environment that `rg.init` works using the three options: providing credentials via function arguments, via environment variables and loading them from the stored file. **Checklist** - [ ] I added relevant documentation - [x] I followed the style guidelines of this project - [x] I did a self-review of my code - [ ] I made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [ ] I filled out [the contributor form](https://tally.so/r/n9XrxK) (see text above) - [x] I have added relevant notes to the `CHANGELOG.md` file (See https://keepachangelog.com/) --------- Co-authored-by: Alvaro Bartolome <alvaro@argilla.io> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Description
This PR includes a new function called
login
that will try to login to the Argilla server using the provided credentials. If it success, the provided credentials will be saved to~/.cache/argilla/credentials.json
file. Theargilla.client.client.Argilla.__init__
method has been updated to try to use the stored credentials if theapi_url
andapi_key
arguments are not provided.Closes #3581
Type of change
How Has This Been Tested
I've added unit tests and tried in a local development environment that
rg.init
works using the three options: providing credentials via function arguments, via environment variables and loading them from the stored file.Checklist
CHANGELOG.md
file (See https://keepachangelog.com/)