Skip to content

Introduce \Icinga\Application\Hook\Essentials#5474

Open
Al2Klimov wants to merge 4 commits intomainfrom
hookEssentials
Open

Introduce \Icinga\Application\Hook\Essentials#5474
Al2Klimov wants to merge 4 commits intomainfrom
hookEssentials

Conversation

@Al2Klimov
Copy link
Copy Markdown
Member

@Al2Klimov Al2Klimov commented Feb 16, 2026

No description provided.

@Al2Klimov Al2Klimov requested a review from lippserd February 16, 2026 15:09
@Al2Klimov Al2Klimov self-assigned this Feb 16, 2026
@cla-bot cla-bot bot added the cla/signed label Feb 16, 2026
@Al2Klimov Al2Klimov marked this pull request as draft February 16, 2026 15:33
@Al2Klimov Al2Klimov marked this pull request as ready for review February 16, 2026 15:45
@Al2Klimov Al2Klimov removed their assignment Feb 16, 2026
Copy link
Copy Markdown
Member Author

@Al2Klimov Al2Klimov left a comment

Choose a reason for hiding this comment

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

Test

  1. Install Fedora 42
  2. Apply https://github.com/Al2Klimov/ansible-icinga-fedora
  3. Downgrade to https://git.icinga.com/packages/icingaweb/-/jobs/817857
  4. Ensure /icingaweb2/health shows Icinga DB and Redis
  5. In /usr/share/icingaweb2/modules/icingadb/run.php, comment out $this->provideHook('health', 'RedisHealth');
  6. Ensure /icingaweb2/health shows Icinga DB only
  7. In /usr/share/icingaweb2/modules/icingadb/run.php, add \Icinga\Module\Icingadb\ProvidedHook\RedisHealth::register();
  8. Ensure /icingaweb2/health shows Icinga DB and Redis again = it works

@Al2Klimov
Copy link
Copy Markdown
Member Author

Al2Klimov commented Mar 4, 2026

@Al2Klimov Al2Klimov force-pushed the hookEssentials branch 2 times, most recently from 0fd0f9d to 9b20fcd Compare March 4, 2026 14:45
@jrauh01 jrauh01 mentioned this pull request Mar 5, 2026
@Al2Klimov Al2Klimov removed the request for review from lippserd March 26, 2026 13:26
@Al2Klimov Al2Klimov marked this pull request as draft March 26, 2026 13:26
@Al2Klimov Al2Klimov self-assigned this Mar 26, 2026
@Al2Klimov Al2Klimov marked this pull request as ready for review March 26, 2026 14:25
Copy link
Copy Markdown
Contributor

@sukhwinder33445 sukhwinder33445 left a comment

Choose a reason for hiding this comment

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

Please rebase.

@@ -0,0 +1,57 @@
<?php
/* Icinga Web 2 | (c) 2026 Icinga GmbH | GPLv2+ */
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please replace:

Suggested change
/* Icinga Web 2 | (c) 2026 Icinga GmbH | GPLv2+ */
// SPDX-FileCopyrightText: 2026 Icinga GmbH <https://icinga.com>
// SPDX-License-Identifier: GPL-3.0-or-later

*
* @return bool
*/
public static function registered(): bool
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would prefer isRegistred as method name.

}

/**
* Return all instances of the hook
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Get all instances of the hook

abstract protected static function getHookName(): string;

/**
* Whether someone registered itself for the hook
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

someone? Get whether the hook is registred fits better.

@lippserd lippserd self-requested a review April 13, 2026 10:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants