-
Notifications
You must be signed in to change notification settings - Fork 28
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
fix(gorgone): Add mbi legacy function to manage host template recursivity #4130
fix(gorgone): Add mbi legacy function to manage host template recursivity #4130
Conversation
WalkthroughThe recent updates in Changes
Poem
Tip Early Access Features
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
5080afa
to
36c4b6b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- centreon-gorgone/gorgone/modules/centreon/mbi/libs/centreon/Host.pm (2 hunks)
Files skipped from review due to trivial changes (1)
- centreon-gorgone/gorgone/modules/centreon/mbi/libs/centreon/Host.pm
centreon-gorgone/gorgone/modules/centreon/mbi/libs/centreon/Host.pm
Outdated
Show resolved
Hide resolved
centreon-gorgone/gorgone/modules/centreon/mbi/libs/centreon/Host.pm
Outdated
Show resolved
Hide resolved
centreon-gorgone/gorgone/modules/centreon/mbi/libs/centreon/Host.pm
Outdated
Show resolved
Hide resolved
36c4b6b
to
7c4c320
Compare
7c4c320
to
1ac3cc9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- centreon-gorgone/gorgone/modules/centreon/mbi/libs/centreon/Host.pm (2 hunks)
Additional comments not posted (6)
centreon-gorgone/gorgone/modules/centreon/mbi/libs/centreon/Host.pm (6)
33-43
: Constructornew
appears to be correctly setting up an instance of the class with optionalcentstorage
database connection. The formatting is clean and the logic is straightforward.
48-49
: The methodsetEtlProperties
is simple and correctly assigns ETL properties to the instance. No issues found here.
111-121
: The methodloadAllHosts
is well-implemented with clear logic for loading hosts and their template relations. Similar toloadAllCategories
, ensure the SQL query performance is optimized.
249-294
: The methodGetHostTemplateAndCategoryForOneHost
correctly retrieves host templates and their categories. The SQL query is well-formed, but ensure that it is covered by performance tests, especially given its complexity and potential for large result sets.
93-103
: The methodloadAllCategories
correctly queries and processes host categories. The use of hash references to store categories and their relations is appropriate. Ensure that the query performance is acceptable with large datasets.
54-73
: The methodgetAllHosts
has been modified to include a condition based on theactivated
flag. This is a good use of conditional logic to control query behavior. However, ensure that theactivated
flag is being set and used correctly elsewhere in the code.Verification successful
The
activated
flag is used consistently across multiple files to control query behavior based on whether the flag is set or not. The flag is initialized to1
and set to0
if any arguments are passed to the function, which is a common pattern in the provided code snippets.Files and Relevant Code Snippets:
- CentreonService.pm
- The
activated
flag is used to conditionally appendAND service_activate='1'
to the query.- CentreonHost.pm
- The
activated
flag is used to conditionally appendAND host_activate='1'
to the query.- MySQLTables.pm
- The
activated
flag is used in a logging statement to indicate whether partitioning is activated for a table.- Host.pm
- The
activated
flag is used to conditionally appendAND host_activate='1'
to the query.- ETLProperties.pm
- The
activated
flag is used to conditionally appendAND host_activate='1'
to the query.The usage of the
activated
flag appears to be consistent and correct across these files. The flag is used to control the inclusion of certain conditions in SQL queries, ensuring that only active records are retrieved when the flag is set.Conclusion
The
activated
flag is being set and used correctly in the codebase.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the `activated` flag in other parts of the codebase. rg --type perl 'activated'Length of output: 1708
Script:
#!/bin/bash # Extracting the relevant code snippets where the `activated` flag is used in the identified files. # CentreonService.pm rg --type perl --context 5 'activated' centreon/lib/perl/centreon/reporting/CentreonService.pm # CentreonHost.pm rg --type perl --context 5 'activated' centreon/lib/perl/centreon/reporting/CentreonHost.pm # MySQLTables.pm rg --type perl --context 5 'activated' centreon-gorgone/gorgone/modules/centreon/mbi/libs/bi/MySQLTables.pm # Host.pm rg --type perl --context 5 'activated' centreon-gorgone/gorgone/modules/centreon/mbi/libs/centreon/Host.pm # ETLProperties.pm rg --type perl --context 5 'activated' centreon-gorgone/gorgone/modules/centreon/mbi/libs/centreon/ETLProperties.pmLength of output: 5613
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- centreon-gorgone/gorgone/modules/centreon/mbi/libs/centreon/Host.pm (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- centreon-gorgone/gorgone/modules/centreon/mbi/libs/centreon/Host.pm
…vity host category can be added to host template. As host template are recursive (a host template can be a parent of another host template) we need to recursivly check every parent for category when adding host in mbi database. REF:MON-37036
REF:MON-37036
REF:MON-37036
1ac3cc9
to
432c4f4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- centreon-gorgone/gorgone/modules/centreon/mbi/libs/centreon/Host.pm (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- centreon-gorgone/gorgone/modules/centreon/mbi/libs/centreon/Host.pm
…vity (#4130) Refs:MON-37036
Description
host category can be added to host template.
As host template are recursive (a host template can be a parent of another host template) we need to recursivly check every parent for category when adding host in mbi database.
Fixes # Refs:MON-37036
Type of change
Target serie
How this pull request can be tested ?
see jira, need to have a centreon db with one host using a host template, with a host category on the host template
launch /usr/share/centreon-bi/bin/centreonBIETL -r
host category should be present in mbi db.
Checklist
Community contributors & Centreon team