Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adding in bascis for victorops #3

Merged
merged 5 commits into from
Apr 3, 2024
Merged

adding in bascis for victorops #3

merged 5 commits into from
Apr 3, 2024

Conversation

boblad
Copy link
Contributor

@boblad boblad commented Apr 1, 2024

Victorops init for pulling users and teams

@boblad boblad requested a review from wilsonehusin April 2, 2024 19:07
go.mod Outdated
@@ -72,6 +72,7 @@ require (
github.com/russross/blackfriday/v2 v2.1.0 // indirect
github.com/sahilm/fuzzy v0.1.1-0.20230530133925-c48e322e2a8f // indirect
github.com/senseyeio/duration v0.0.0-20180430131211-7c2a214ada46 // indirect
github.com/victorops/go-victorops v1.0.7 // indirect
Copy link
Member

Choose a reason for hiding this comment

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

interesting... this shouldn't be indirect. might need to run go mod tidy, which reminds me i should add that in CI.

@@ -20,10 +20,12 @@ type Pager interface {
PopulateTeamSchedules(ctx context.Context, team *Team) error
}

func NewPager(kind string, apiKey string) (Pager, error) {
func NewPager(kind string, apiKey string, appId string) (Pager, error) {
Copy link
Member

Choose a reason for hiding this comment

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

nice!


func (v *VictorOps) toTeam(team victorops.Team) *Team {
return &Team{
// PagerDuty does not expose a slug, so generate one.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// PagerDuty does not expose a slug, so generate one.
// VictorOps does not expose a slug, so generate one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added this with my last push

}

for _, member := range vmembers.Members {
members = append(members, &User{Resource: Resource{ID: member.Username}})
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't this use toUser as well?

Copy link
Member

Choose a reason for hiding this comment

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

oh i made the bad example on pagerduty for this one huh woops

Comment on lines 74 to 83
vusers, _, err := v.client.GetAllUsers()
if err != nil {
return nil, err
}

for _, userSlice := range vusers.Users {
for _, user := range userSlice {
users = append(users, v.toUser(user))
}
}
Copy link
Member

@wilsonehusin wilsonehusin Apr 2, 2024

Choose a reason for hiding this comment

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

oops I missed this first time around: probably should use GetAllUserV2 here https://pkg.go.dev/github.com/victorops/[email protected]/victorops#Client.GetAllUserV2

Copy link
Member

@wilsonehusin wilsonehusin left a comment

Choose a reason for hiding this comment

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

🚢

@boblad boblad merged commit 2b891bd into main Apr 3, 2024
3 checks passed
@boblad boblad deleted the ALRT-563/victorops branch April 3, 2024 21:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants