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

Use icinga-go-library #114

Merged
merged 3 commits into from
Jun 3, 2024
Merged

Use icinga-go-library #114

merged 3 commits into from
Jun 3, 2024

Conversation

yhabteab
Copy link
Member

@yhabteab yhabteab commented Oct 19, 2023

@yhabteab
Copy link
Member Author

I had to use the new, not yet merged PR as the DB#BuildColumns() was dropped during the migration to the new library and we have no other direct way to access the internal DB column mapper.

query, _ := database.NewQB(eventRow).SetExcludedColumns("id").Insert(db)
eventID, err := database.InsertObtainID(ctx, tx, query, eventRow)

cmd/icinga-notifications-daemon/main.go Outdated Show resolved Hide resolved
cmd/icinga-notifications-daemon/main.go Outdated Show resolved Hide resolved
internal/utils/utils.go Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
@yhabteab yhabteab force-pushed the switch-to-igl branch 2 times, most recently from cf958eb to bba22c5 Compare May 29, 2024 15:34
Copy link
Member

@oxzi oxzi left a comment

Choose a reason for hiding this comment

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

Please feel free removing the now unused utils functions. Otherwise, LGTM.

internal/utils/utils.go Show resolved Hide resolved
@yhabteab yhabteab requested review from oxzi and removed request for Al2Klimov May 31, 2024 09:38
oxzi
oxzi previously approved these changes May 31, 2024
go.mod Outdated

require (
github.com/creasty/defaults v1.7.0
github.com/emersion/go-sasl v0.0.0-20231106173351-e73c9f7bad43
github.com/emersion/go-smtp v0.21.2
github.com/goccy/go-yaml v1.11.3
github.com/google/uuid v1.6.0
github.com/icinga/icingadb v1.1.1-0.20230418113126-7c4b947aad3a
github.com/icinga/icinga-go-library v0.1.1-0.20240529153148-5054d55f8005
Copy link
Collaborator

Choose a reason for hiding this comment

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

I just tagged the new version as v0.2.0, please update this to use the tagged version.

The `context.Context` related changes are neccessary as otherwise you
will have to wait for 5m when having Icinga notification started with
invalid database config due to the internal database.Connect retrials.
Lastly, the logging related changes are useful as we want to properly
render the stack traces when we unexpectedly exit from the main function
instead of wrapping the error in a confusing `zap.Error()` json key.
@julianbrost julianbrost merged commit aa3b0db into main Jun 3, 2024
12 checks passed
@julianbrost julianbrost deleted the switch-to-igl branch June 3, 2024 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla/signed CLA is signed by all contributors of a PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants