a charmdelete command #704

Merged
merged 1 commit into from Jan 30, 2017

Conversation

Projects
None yet
7 participants
Member

jrwren commented Jan 26, 2017

No description provided.

Member

jujugui commented Jan 26, 2017

Refer to this link for build results (access rights to CI server needed):
http://ci-cge.jujugui.org:8080//job/charmstore/2020/

Owner

urosj commented Jan 26, 2017

@jrwren please add a caveat ... if it's promulgated, it can't be deleted. only non-promulgated can be.

Member

jujugui commented Jan 27, 2017

Refer to this link for build results (access rights to CI server needed):
http://ci-cge.jujugui.org:8080//job/charmstore/2021/

💯

Member

jrwren commented Jan 27, 2017

test this please

Member

jujugui commented Jan 27, 2017

Refer to this link for build results (access rights to CI server needed):
http://ci-cge.jujugui.org:8080//job/charmstore/2022/

LGTM with a few comments

cmd/charmdelete/main.go
+ "gopkg.in/mgo.v2/bson"
+)
+
+var logger = loggo.GetLogger("essync")
@mhilton

mhilton Jan 30, 2017

Member

not essync

cmd/charmdelete/main.go
+var (
+ index = flag.String("index", "cs", "Name of index to charmDelete.")
+ loggingConfig = flag.String("logging-config", "", "specify log levels for modules e.g. <root>=TRACE")
+ user = flag.String("user", "", "Delete all charms for a user. Do not mix with charmMatch.")
@mhilton

mhilton Jan 30, 2017

Member

This requires "go fmt"ing

cmd/charmdelete/main.go
+ }
+ }
+ if err := run(flag.Arg(0)); err != nil {
+ logger.Errorf("cannot charmDelete elasticsearch: %v", err)
@mhilton

mhilton Jan 30, 2017

Member

I don't understand what this error means

cmd/charmdelete/main.go
+ return nil
+}
+
+func deleteE(entity *mongodoc.Entity, store *charmstore.Store) {
@mhilton

mhilton Jan 30, 2017

Member

make this deleteEntity to make it clear what it does.

Member

jujugui commented Jan 30, 2017

Refer to this link for build results (access rights to CI server needed):
http://ci-cge.jujugui.org:8080//job/charmstore/2023/

bac approved these changes Jan 30, 2017

Thanks @jrwren this will be nice to have. A few comments, take them as you will :)

+ query := bson.D{{"user", *user}}
+ if *user == "" {
+ fmt.Printf("using match _id $regex %s\n", *charmMatch)
+ query = bson.D{{"_id", bson.D{{"$regex", *charmMatch}}}}
@hatched

hatched Jan 30, 2017

Owner

Because this could match charms the admin doesn't intend it should probably print a list of ids that match and then a confirmation step. I see that there is a dryrun flag but it's not the default for this command, maybe it should be?

+ counter := 0
+ for iter.Next(&entity) {
+ if entity.PromulgatedURL != nil && !*deletePromulgated {
+ fmt.Printf("not deleting promulgated charm %s\n", entity.URL)
@hatched

hatched Jan 30, 2017

Owner

Seeing this log I'd ask why...maybe something like "entity.URL is promulgated, not deleting" is more appropriate.

Member

jrwren commented Jan 30, 2017

:shipit:

Member

jujugui commented Jan 30, 2017

Status: merge request accepted. Url: http://ci-gce.jujugui.org:8080/job/charmstore-merge

@jujugui jujugui merged commit 74f02d0 into juju:v5-unstable Jan 30, 2017

1 check passed

default Build finished.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment