Skip to content
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,12 @@ All notable changes to `src-cli` are documented in this file.

### Changed

- Renamed `src users clean` command to `src users prune` [#901](https://github.com/sourcegraph/src-cli/pull/901)

### Fixed

- Fix network timeout in `src users clean` occuring in instances with many users [#901](https://github.com/sourcegraph/src-cli/pull/901)

### Removed

## 4.3.0
Expand Down
10 changes: 9 additions & 1 deletion cmd/src/users.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ The commands are:
get gets a user
create creates a user account
delete deletes a user account
clean deletes inactive users
prune deletes inactive users
tag add/remove a tag on a user

Use "src users [command] -h" for more information about a command.
Expand Down Expand Up @@ -90,3 +90,11 @@ type UserUsageStatistics struct {
LastActiveTime string
LastActiveCodeHostIntegrationTime string
}

type SiteUser struct {
ID string
Username string
Email string
SiteAdmin bool
LastActiveAt string
}
71 changes: 39 additions & 32 deletions cmd/src/users_clean.go → cmd/src/users_prune.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,21 @@ This command removes users from a Sourcegraph instance who have been inactive fo

Examples:

$ src users clean -days 182
$ src users prune -days 182
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How are such command/API changes usually made in src cli? Should this be a major version increase or should we keep backward compatibility leaving both clean and prune commands and marking one deprecated?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost all the functionality changes in terms of the command run locally, the old clean command had some big timeout issues see #848. The name change from clean to prune was mostly just a style change. I don't think this command is being widely used so I think the change is alright. To my knowledge no one has put this on a cronjob or anything like that yet.


$ src users clean -remove-admin -remove-never-active
$ src users prune -remove-admin -remove-null-users
`

flagSet := flag.NewFlagSet("clean", flag.ExitOnError)
flagSet := flag.NewFlagSet("prune", flag.ExitOnError)
usageFunc := func() {
fmt.Fprintf(flag.CommandLine.Output(), "Usage of 'src users %s':\n", flagSet.Name())
flagSet.PrintDefaults()
fmt.Println(usage)
}
var (
daysToDelete = flagSet.Int("days", 60, "Days threshold on which to remove users, must be 60 days or greater and defaults to this value ")
removeAdmin = flagSet.Bool("remove-admin", false, "clean admin accounts")
removeNoLastActive = flagSet.Bool("remove-never-active", false, "removes users with null lastActive value")
removeAdmin = flagSet.Bool("remove-admin", false, "prune admin accounts")
removeNoLastActive = flagSet.Bool("remove-null-users", false, "removes users with no last active value")
skipConfirmation = flagSet.Bool("force", false, "skips user confirmation step allowing programmatic use")
apiFlags = api.NewFlags(flagSet)
)
Expand All @@ -51,7 +51,7 @@ Examples:
client := cfg.apiClient(apiFlags, flagSet.Output())

currentUserQuery := `
query {
query getCurrentUser {
currentUser {
username
}
Expand All @@ -68,43 +68,50 @@ query {
return err
}

usersQuery := `
query Users() {
users() {
nodes {
...UserFields
}
}
getInactiveUsersQuery := `
query getInactiveUsers {
site {
users {
nodes {
username
email
siteAdmin
lastActiveAt
}
}
}
Comment on lines +73 to +82
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intendation is off

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is hitting the aggregated_user_statistics table, and the value of lastActiveAt doesn't need to access the event_logs table the same way usageStatistics used to. Try it out here, its much more performant.

aggregated_user_statistics table is updating in the background, so this isn't accessing the event logs table at the time of the request.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}
` + userFragment
`

// get users to delete
var usersResult struct {
Users struct {
Nodes []User
Site struct {
Users struct {
Nodes []SiteUser
}
}
}
if ok, err := client.NewRequest(usersQuery, nil).Do(ctx, &usersResult); err != nil || !ok {

if ok, err := client.NewRequest(getInactiveUsersQuery, nil).Do(ctx, &usersResult); err != nil || !ok {
return err
}

usersToDelete := make([]UserToDelete, 0)
for _, user := range usersResult.Users.Nodes {
daysSinceLastUse, wasLastActive, err := computeDaysSinceLastUse(user)
for _, user := range usersResult.Site.Users.Nodes {
daysSinceLastUse, hasLastActive, err := computeDaysSinceLastUse(user)
if err != nil {
return err
}
// never remove user issuing command
if user.Username == currentUserResult.Data.CurrentUser.Username {
continue
}
if !wasLastActive && !*removeNoLastActive {
if !hasLastActive && !*removeNoLastActive {
continue
}
if !*removeAdmin && user.SiteAdmin {
continue
}
if daysSinceLastUse <= *daysToDelete && wasLastActive {
if daysSinceLastUse <= *daysToDelete && hasLastActive {
continue
}
deleteUser := UserToDelete{user, daysSinceLastUse}
Expand Down Expand Up @@ -146,13 +153,13 @@ query Users() {
}

// computes days since last usage from current day and time and UsageStatistics.LastActiveTime, uses time.Parse
func computeDaysSinceLastUse(user User) (timeDiff int, wasLastActive bool, _ error) {
// handle for null lastActiveTime returned from
if user.UsageStatistics.LastActiveTime == "" {
wasLastActive = false
return 0, wasLastActive, nil
func computeDaysSinceLastUse(user SiteUser) (timeDiff int, hasLastActive bool, _ error) {
// handle for null LastActiveAt returned from
if user.LastActiveAt == "" {
hasLastActive = false
return 0, hasLastActive, nil
}
timeLast, err := time.Parse(time.RFC3339, user.UsageStatistics.LastActiveTime)
timeLast, err := time.Parse(time.RFC3339, user.LastActiveAt)
if err != nil {
return 0, false, err
}
Expand All @@ -162,7 +169,7 @@ func computeDaysSinceLastUse(user User) (timeDiff int, wasLastActive bool, _ err
}

// Issue graphQL api request to remove user
func removeUser(user User, client api.Client, ctx context.Context) error {
func removeUser(user SiteUser, client api.Client, ctx context.Context) error {
query := `mutation DeleteUser($user: ID!) {
deleteUser(user: $user) {
alwaysNil
Expand All @@ -178,7 +185,7 @@ func removeUser(user User, client api.Client, ctx context.Context) error {
}

type UserToDelete struct {
User User
User SiteUser
DaysSinceLastUse int
}

Expand All @@ -189,8 +196,8 @@ func confirmUserRemoval(usersToRemove []UserToDelete) (bool, error) {
t.SetOutputMirror(os.Stdout)
t.AppendHeader(table.Row{"Username", "Email", "Days Since Last Active"})
for _, user := range usersToRemove {
if len(user.User.Emails) > 0 {
t.AppendRow([]interface{}{user.User.Username, user.User.Emails[0].Email, user.DaysSinceLastUse})
if user.User.Email != "" {
t.AppendRow([]interface{}{user.User.Username, user.User.Email, user.DaysSinceLastUse})
t.AppendSeparator()
} else {
t.AppendRow([]interface{}{user.User.Username, "", user.DaysSinceLastUse})
Expand Down