search

civo / cli-rubygem

Command line interface for interacting with Civo's cloud API
https://www.civo.com
MIT License
15 stars 4 forks source link

Automatically set default apikeys #43

Closed AndrewCsoka closed 4 years ago

AndrewCsoka commented 5 years ago

This is intended to fix https://github.com/civo/cli/issues/41 and also handle the inverse case: when a user deletes all but one key, the last remaining key should become the default.

An additional behaviour is possible: when the user deletes the existing default, you could randomly assign a new default, but I think that could be confusing.

I've raised this as a draft PR, since it's unbuilt and untested so far. I'm also unsure about the keys data structure so keys.keys.key here might be wrong.

Feel free to comment. I've never used Ruby.

AndrewCsoka commented 5 years ago

I've built and tested locally. The changes work as I'd expect. Ready for review. @kaihoffman

kaihoffman commented 5 years ago

I think @andyjeffries had some reservations about automatically setting an API key on removal. From a previous (now-closed) PR:

civo apikeys
# development  <======
# production

civo apikey delete development
# Deletes development
# Has no current, so resets it to the first one, e.g. production
# But the current before was development so the user then does...

civo instance delete super-important-database
andyjeffries commented 5 years ago

I think I'm now OK with setting a default on deletion of an API key (my mind has changed), provided we make it very clear to the user what the new default key is (colourised).

andyjeffries commented 5 years ago

I'm even OK with doing it if you have 3 keys and delete 1, it picking the first of the remaining two, providing it communicates this to the user.

andyjeffries commented 4 years ago

@kaihoffman how does that fit in with your other PR that was merged this morning? Complimentary/conflicting? It says conflicting above, but I mean on a business logic level.

kaihoffman commented 4 years ago

@andyjeffries the other PR also does this behaviour. The conflict arises from a different implementation of the same logic. The other PR also extends this slightly, and informs the user of changes when a current key gets deleted etc.

Closing for now as superseded. Hope that's OK?