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

Allow disabling recommendations using a CMDB class #2862

Closed

Conversation

vpodzime
Copy link
Contributor

@vpodzime vpodzime commented Apr 2, 2024

CMDB classes default to the data namespace. So if the user adds cfengine_recommendations_disabled as a class in their CMDB data for the hub machine, checking the class in the default namespace is not good enough to disable recommendations.

CMDB classes default to the `data` namespace. So if the user adds
`cfengine_recommendations_disabled` as a class in their CMDB
data for the hub machine, checking the class in the default
namespace is not good enough to disable recommendations.
Copy link
Member

@nickanderson nickanderson left a comment

Choose a reason for hiding this comment

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

I would look to improve this in some different way.

perhaps one or more of:

  • Emit a report that tells people to define default:cfengine_recommendations_disabled
  • Update the docs to reference default:
  • Improve CMDB UI

@@ -667,7 +667,7 @@ bundle common def
expression => some( ".+", "def.control_executor_runagent_socket_allow_users" );

"cfengine_recommendations_enabled"
expression => "!cfengine_recommendations_disabled";
expression => "!cfengine_recommendations_disabled.!data:cfengine_recommendations_disabled";
Copy link
Member

Choose a reason for hiding this comment

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

I am not a big fan of having multiple classes that do the same thing generally. This class is suggested because it's not clear how to define the class in MP CMDB UI, seems to me better to improve this elsewhere than add another class.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I completely agree. This is really just my "user story" persona trying to scratch my own itch.

@nickanderson
Copy link
Member

I would look to improve this in some different way.

perhaps one or more of:

* Emit a report that tells people to define `default:cfengine_recommendations_disabled`

* Update the docs to reference `default:`

* Improve CMDB UI

@vpodzime what do you think about this proposal? #2863
It does two of the suggested things above.

@vpodzime
Copy link
Contributor Author

vpodzime commented Apr 3, 2024

@vpodzime what do you think about this proposal? #2863
It does two of the suggested things above.

Let's continue with the discussion there.

@vpodzime vpodzime closed this Apr 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants