CLOUDP-329800: reuse logout and deprecate config delete #4112
Conversation
github-actions
bot
added
the
need-doc-review
|
APIx Bot |
: a message has been sent to Docs Slack channel 🚀.
| Aliases: []string{"rm"}, | ||
| Short: "Delete a profile.", | ||
| Args: require.ExactArgs(1), | ||
| Deprecated: "Please use the 'atlas auth logout' command instead.", |
There was a problem hiding this comment.
Q: if the page is deleted, will users actually see this deprecation message? perhaps it is displayed in help output? Usually when we deprecate features we keep the page around for a while but add a banner so users get advance guidance on how they need to adjust their workflows accordingly.
My recommendation would be to keep this page and we can add a banner on our end to give users a heads-up. What are your thoughts?
There was a problem hiding this comment.
Hey @jwilliams-mongo , yes the deprecation warning would show up on call and on help.
Unfortunately, the removal is inherited from the cobra framework, I'll check if they have any way to override it but if they don't it might not be worth the effort to keep the page.
There was a problem hiding this comment.
yeah if it's too much effort I'm fine with moving forward with the removal
There was a problem hiding this comment.
I checked it and it seems that the removal happens at cobra level. I'll chat w/ the team to see if we can find a workaround, but meanwhile I think we'll keep it this way
| return err | ||
| } | ||
| // Execute: atlas auth logout --profile <profile-name> --force | ||
| cmd := exec.Command(executable, "auth", "logout", "--profile", opts.Entry, "--force") |
There was a problem hiding this comment.
Why --force?
Both commands prompt for confirmation. I think --force should only be passed if the user has set the flag
There was a problem hiding this comment.
I'm keeping the config prompt, so to avoid two prompts I'm using force in the logout one, but I could remove the prompt from config and use logout, no strong opinion
There was a problem hiding this comment.
Aha makes sense. No preference either on which command prompts. Thanks for explanation!
There was a problem hiding this comment.
let's remove prompt and blindly use the logout one, in case we want to update prompts we just update one command
There was a problem hiding this comment.
I'll address this in a follow up PR
| // Configuration needs to be deleted from toml, as viper doesn't support this yet. | ||
| // FIXME :: change when https://github.com/spf13/viper/pull/519 is merged. | ||
| settings := viper.AllSettings() | ||
| settings := s.viper.AllSettings() |
There was a problem hiding this comment.
bugfix - without this, we were deleting the whole profile on logout
Proposed changes
Jira ticket: CLOUDP-329800
noAuthas authMechanism to identify unauthenticated users (manually setup profiles)Checklist
make fmtand formatted my codeFurther comments