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

feat: add update to auth cmd #450

Merged
merged 13 commits into from
Jun 13, 2023

Conversation

HatiCode
Copy link
Contributor

@HatiCode HatiCode commented May 22, 2023

Closes #388

📑 Description

This PR adds an update subcommand to the auth command, allowing to update a key in a previously configured provider e.g: password, model...
It also fixes a couple typos met along the way.

✅ Checks

  • My pull request adheres to the code style of this project
  • My code requires changes to the documentation
  • I have updated the documentation as required
  • All the tests have passed

ℹ Additional Information

Signed-off-by: Alexandre Steppé <alexandre.steppe@gmail.com>
Signed-off-by: Alexandre Steppé <alexandre.steppe@gmail.com>
@HatiCode HatiCode changed the title feat/add update to auth cmd feat: add update to auth cmd May 22, 2023
@matthisholleville
Copy link
Contributor

Hello, thank you very much for your contribution. I just reviewed your PR. It seems like it implements an update method that requires setting all the fields. Imagine if the user wants to modify only the password in their configuration or the model without having to fill in all the other fields. I think it would be interesting to lean more towards this implementation.

What do you think ?

@HatiCode
Copy link
Contributor Author

@matthisholleville indeed this is what I'm trying to achieve, now because I reused the viper.Set() method at line 74, it reimplements all default empty values from the flags declared alongside the changes one. Do you know how I could keep the change passed with a flag and leave the rest untouched ?

@matthisholleville
Copy link
Contributor

Check if the flag is not empty? I don't think flags should have default values in the case of an update.

Signed-off-by: Alexandre Steppé <alexandre.steppe@gmail.com>
Signed-off-by: Alexandre Steppé <alexandre.steppe@gmail.com>
@HatiCode HatiCode marked this pull request as ready for review May 26, 2023 19:31
@HatiCode HatiCode requested review from a team as code owners May 26, 2023 19:31
@HatiCode
Copy link
Contributor Author

@matthisholleville I made a couple changes and it's now working, changes are only applied for the flags used. I still feel like this could use some nice refactoring at a certain point tho

@matthisholleville
Copy link
Contributor

Hello @HatiCode can you please provide an example of the commande ?

@HatiCode
Copy link
Contributor Author

HatiCode commented Jun 1, 2023

@matthisholleville
Let's say I want to update the model I use for openai :
k8sgpt auth update [backend name] [flags] -> k8sgpt auth update openai -m gpt-2

Screenshot 2023-06-01 at 18 25 07 Screenshot 2023-06-01 at 18 25 41 Screenshot 2023-06-01 at 18 26 28 Screenshot 2023-06-01 at 18 27 16 Screenshot 2023-06-01 at 18 27 38

@AlexsJones AlexsJones merged commit 01aeeb3 into k8sgpt-ai:main Jun 13, 2023
@HatiCode HatiCode deleted the feat/add-update-to-auth-cmd branch June 13, 2023 12:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[Feature]: Enhance Auth command
3 participants