Add abra app remove command #43
Closed
knoflook
wants to merge 7 commits from
knoflook/abra:dev into main
pull from: knoflook/abra:dev
merge into: :main
:main
:fix/492
:renovate/golang-1.26
:local-integration-testing
:renovate/github.com-charmbracelet-lipgloss-2.x
:renovate/otel-weaver-0.x
:renovate/codespell-2.x
:renovate/github.com-charmbracelet-bubbletea-2.x
:renovate/tonistiigi-xx-1.x
:renovate/alpine-3.x
:renovate/github.com-charmbracelet-log-2.x
:chore-deps
:fix/deps
:fix/613
Labels
Clear labels
bug
build
ci/cd
critical fix
design
documentation
duplicate
easy-first-issue
enhancement
help wanted
i10n
i18n
installer
invalid
question
release
release-candidate
security
tech-debt
test
wontfix
Something is not working
go build related issues
Building things with CI/CD
https://docs.coopcloud.tech/federation/resolutions/passed/010/
UI/UX
Documenting all the things
This issue or pull request already exists
Something for new people to get stuck into. We hope it's easy!
New feature
Need some help
Everything to do with localisation
Everything to do with internationalisation
Everything to do with the install script.
Something is wrong
More information is needed
Release management
Related to the new release candidate
Security related
Unit/integration testing
This won't be fixed
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
3wordchant
aadil (Aadil Ayub)
abra-bot (Abra Bot)
ammaratef45
amras (Sarma)
Apfelwurm
BornDeleuze
Brooke
carla
cas (Cassowary)
coopcloud
cyrnel
decentral1se (d1)
dede
devydave
fauno (fauno)
iexos
jade (Jade Ambrose)
jjsfunhouse
jmakdah2 (Jackie Makdah)
joe-irving (Joe Irving)
kawaiipunk (KawaiiPunk)
knoflook
kolaente
lambdabundesverband
linnealovespie (April)
moosemower
moritz
notplants
oxaliq (sorrel)
p4u1
pharaohgraphy (Andrew 🐦🔥❤️🔥✴️)
renovate-bot (Comrade Renovate Bot)
ripclap
simon
sixsmith (Sixsmith)
stevensting
trav (Trav Fryer)
val (val (he/him))
yksflip
Clear assignees
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: toolshed/abra#43
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
When ran with no flags, it removes the env file and lets you choose which secrets to remove. If you add --volumes, it also lets you choose which volumes should be removed.
If you only specify --force, the env file and all secrets will be removed
If you specify --force --volumes, the volumes will also be removed.
Nice one! Buncha comments, hope it is not overwhelming 🙈
Can we have it remove env file, secrets and volumes?
Thanks 🚀
(hmu on matrix if anything is unclear)
@@ -17,1 +28,4 @@var Force boolvar ForceFlag = &cli.BoolFlag{This is a general flag we'll add to other sub-commands, it can live in cli/internal/common.go?
@@ -23,0 +42,4 @@ForceFlag,},Action: func(c *cli.Context) error {// Check if app name was provided by userWe haven't discussed this but the existing convention that I see is that we don't add comments to code to explain what it is doing unless it is doing something strange or difficult. Otherwise, we comment to explain why we're doing something (context, not what it is doing).
The issue is, that when the code changes, you might forget to change the comment and then the next person might be caught reading the comment which describes code that doesn'te exist anymore. I would say have a look around the rest of the codebase at comment usage and perhaps remove the obvious ones in this function.
For example the
// we have to map the names to ID'scomment is a good one, I'd keep that.@@ -23,0 +43,4 @@},Action: func(c *cli.Context) error {// Check if app name was provided by userAppName := c.Args().First()appName(e.g. host)@@ -23,0 +51,4 @@if !Force {response := falseprompt := &survey.Confirm{Message: "About to delete " + AppName + ", are you sure?",Message: fmt.Sprintf("About to delete %s, are you sure?", appName)@@ -23,0 +54,4 @@Message: "About to delete " + AppName + ", are you sure?",}survey.AskOne(prompt, &response)if response == false {@@ -23,0 +59,4 @@}}// Get app listAppFiles, err := config.LoadAppFiles("")appFiles, err := config.LoadAppFiles()Can't do it that way, if you remove "" LoadAppFiles returns an empty map[]
@@ -23,0 +61,4 @@// Get app listAppFiles, err := config.LoadAppFiles("")if err != nil {return errlogrus.Fatal(err)@@ -23,0 +63,4 @@if err != nil {return err}AppPath := AppFiles[AppName].PathappPath := AppFiles[appName].Path@@ -23,0 +73,4 @@}// Remove the fileerr = os.Remove(AppPath)if err != nil {@@ -23,0 +84,4 @@fs.Add("name", AppName)SecretList, err := cl.SecretList(ctx, types.SecretListOptions{Filters: fs})if err != nil {return errlogrus.Fatal(err)@@ -23,0 +108,4 @@// Actually remove the secretsfor _, name := range SecretNamesToRemove {// DEBUG: SecretIDsToRemove = append(SecretIDsToRemove, Secrets[name])Let's remove that 🙃
@@ -23,0 +110,4 @@for _, name := range SecretNamesToRemove {// DEBUG: SecretIDsToRemove = append(SecretIDsToRemove, Secrets[name])err := cl.SecretRemove(ctx, Secrets[name])if err != nil {@@ -23,0 +121,4 @@VolumeListOKBody, err := cl.VolumeList(ctx, fs)VolumeList := VolumeListOKBody.Volumesif err != nil {return errlogrus.Fatal(err)@@ -23,0 +128,4 @@Vols = append(Vols, vol.Name)}// Remove the volumes if desiredif Volumes == true {if Volumes {Lol accidentally clicked that approve button but anyway, you get the idea 🙈
In the image: discussion about
--forcebehaviour without--volumesspecifiedMerged in
6732edf8db🥳(some
git rebase-foo which we can co-work on figuring out next time)Also took a pass in
36af302d5fto drop some "dangling else" stuff, down to chat about that also.Thanks so much for this!
Pull request closed