Drop some superfluous Redis -> DB value mappings by yhabteab · Pull Request #964 · Icinga/icingadb

yhabteab · 2025-05-21T10:01:26Z

This PR is basically the same as Icinga/icinga2#10452 and removes the now superfluous Redis -> DB value mappings, and also makes the {host,service}_state.is_acknowledged column a simple boolenum and introduces a new is_sticky_acknowledgement column to indicate sticky acknowledgements.

Requires

IcingaDB: Make Redis & DB values consistent icinga2#10452

schema/pgsql/upgrades/1.4.0.sql

oxzi

First, thanks for this PR. In general, it looks good.

I went over the code - and its counterpart in Icinga 2 - multiple times and added some small comments.

Furthermore, I did some testing in a playground setup and clicking a non-sticky and a sticky in Icinga DB Web resulted in the expected data within the relation database.

MariaDB [icingadb]> select comment.text, is_acknowledged, is_sticky_acknowledgement  from service_state join comment on service_state.acknowledgement_comment_id = comment.id where is_acknowledged = 'y'\G
*************************** 1. row ***************************
                     text: F
          is_acknowledged: y
is_sticky_acknowledgement: n
*************************** 2. row ***************************
                     text: sticky F
          is_acknowledged: y
is_sticky_acknowledgement: y
2 rows in set (0.004 sec)

cmd/icingadb-migrate/convert.go

pkg/icingadb/history/sla.go

cmd/icingadb-migrate/convert.go

oxzi

Looks good so far, thanks!

After this PR's counterpart against Icinga 2 was properly reviewed and merged, I would take another look. Consider this a soft approve.

However, my comment regarding the different column order in PostgreSQL remains; #964 (comment). I would like to hear some opinions if we should resolve this or treat this as an annoyance to be ignored.

tests/history_test.go

pkg/icingadb/v1/history/comment.go

tests/history_test.go

pkg/icingadb/v1/history/comment.go

tests/object_sync_test.go

pkg/icingadb/v1/history/comment.go

tests/object_sync_test.go

yhabteab · 2025-06-04T13:13:24Z

That was my final push :). So please have another look again!

pkg/icingadb/v1/history/state.go

tests/object_sync_test.go

pkg/icingadb/v1/history/notification.go

Icinga 2 sets now the `state_type` field to either `hard` or `soft`, thus the mappings within the Icinga DB code is no longer needed.

…ype` formats

oxzi

LGTM.

Went over the code another time, tested it together with Icinga 2 based on your other PR, tested updated icingadb-migrate command. Nothing fell apart. Thanks!

oxzi

🎉

cla-bot bot added the cla/signed label May 21, 2025

yhabteab requested a review from oxzi May 21, 2025 10:01

yhabteab force-pushed the streamline-redis-and-db-types branch from 9f60050 to 85b55ab Compare May 21, 2025 10:08

yhabteab mentioned this pull request May 21, 2025

IcingaDB: Make Redis & DB values consistent Icinga/icinga2#10452

Merged

oxzi added this to the 1.4.0 milestone May 21, 2025

oxzi reviewed May 21, 2025

View reviewed changes

schema/pgsql/upgrades/1.4.0.sql Show resolved Hide resolved

oxzi self-requested a review May 21, 2025 14:26

oxzi reviewed May 23, 2025

View reviewed changes

yhabteab force-pushed the streamline-redis-and-db-types branch from 85b55ab to ca18d4c Compare May 23, 2025 09:29

yhabteab requested a review from oxzi May 23, 2025 09:30

oxzi reviewed May 23, 2025

View reviewed changes

yhabteab force-pushed the streamline-redis-and-db-types branch from ca18d4c to 3a20d8d Compare May 28, 2025 10:35

julianbrost reviewed May 28, 2025

View reviewed changes

tests/history_test.go Outdated Show resolved Hide resolved

tests/history_test.go Outdated Show resolved Hide resolved

pkg/icingadb/v1/history/comment.go Outdated Show resolved Hide resolved

yhabteab force-pushed the streamline-redis-and-db-types branch 2 times, most recently from dd51836 to 7b2fd34 Compare May 30, 2025 10:29

yhabteab requested a review from julianbrost May 30, 2025 10:35

julianbrost reviewed May 30, 2025

View reviewed changes

tests/object_sync_test.go Outdated Show resolved Hide resolved

pkg/icingadb/v1/history/comment.go Outdated Show resolved Hide resolved

julianbrost reviewed May 30, 2025

View reviewed changes

tests/object_sync_test.go Outdated Show resolved Hide resolved

yhabteab force-pushed the streamline-redis-and-db-types branch 2 times, most recently from 5026371 to 7fdc513 Compare May 30, 2025 15:33

oxzi added the area/migrate cmd/icingadb-migrate label Jun 4, 2025

yhabteab force-pushed the streamline-redis-and-db-types branch 2 times, most recently from d15e93d to 0b5af71 Compare June 4, 2025 12:30

yhabteab requested review from julianbrost and oxzi and removed request for julianbrost and oxzi June 4, 2025 12:32

yhabteab force-pushed the streamline-redis-and-db-types branch 2 times, most recently from 50f67f6 to 7cf3c9c Compare June 4, 2025 13:12

yhabteab requested review from julianbrost and oxzi June 4, 2025 13:13

julianbrost reviewed Jun 5, 2025

View reviewed changes

pkg/icingadb/v1/history/state.go Outdated Show resolved Hide resolved

tests/object_sync_test.go Outdated Show resolved Hide resolved

pkg/icingadb/v1/history/notification.go Show resolved Hide resolved

Make is_acknowledged a boolenum & add is_sticky_acknowledgement

b32379a

yhabteab force-pushed the streamline-redis-and-db-types branch from 7cf3c9c to c7da5ed Compare June 5, 2025 12:23

Streamline Redis & DB state_type values

9e23399

Icinga 2 sets now the `state_type` field to either `hard` or `soft`, thus the mappings within the Icinga DB code is no longer needed.

yhabteab force-pushed the streamline-redis-and-db-types branch from c7da5ed to c6a569e Compare June 5, 2025 12:25

yhabteab added 3 commits June 5, 2025 14:35

Move CommentType to history package & support both `comment.entry_t…

c4a69d7

…ype` formats

Drop superfluous {user,notifications},states,types filter mappings

40a4bcc

Drop superfluous NotificationType mappings

ea17fa4

yhabteab force-pushed the streamline-redis-and-db-types branch from c6a569e to ea17fa4 Compare June 5, 2025 12:35

yhabteab requested a review from julianbrost June 5, 2025 13:22

julianbrost previously approved these changes Jun 6, 2025

View reviewed changes

oxzi previously approved these changes Jun 6, 2025

View reviewed changes

tests: fix SQL syntax error with postgres adapter

6954dc4

yhabteab dismissed stale reviews from oxzi and julianbrost via 6954dc4 June 10, 2025 12:16

yhabteab requested review from julianbrost and oxzi June 10, 2025 12:18

oxzi approved these changes Jun 10, 2025

View reviewed changes

julianbrost approved these changes Jun 10, 2025

View reviewed changes

julianbrost merged commit 5c01c2c into main Jun 10, 2025
31 checks passed

julianbrost deleted the streamline-redis-and-db-types branch June 10, 2025 12:38

Conversation

yhabteab commented May 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Requires

Uh oh!

Uh oh!

oxzi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

oxzi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

yhabteab commented Jun 4, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

oxzi left a comment

Choose a reason for hiding this comment

Uh oh!

oxzi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

yhabteab commented May 21, 2025 •

edited

Loading