ohai icon indicating copy to clipboard operation
ohai copied to clipboard

Safe attribute creation

Open mcquin opened this issue 10 years ago • 8 comments

Currently, plugin authors have to write attribute Mash.new unless attribute to prevent previous data set on attribute from being overwritten. Plugin authors also have to ensure that existing plugins do not accidentally overwrite attribute when writing plugins that add data to attribute.

It would be nice if attributes could be created automatically and safely. We can harness the use of provides to make this happen:

  • If a plugin says provides "attr" then
    • If attr exists in ohai data, do nothing
    • Else, set attr to nil
  • If a plugin says provides "nested/attr" then
    • If nested exists in ohai data,
      • If attr is a subkey of nested, do nothing,
      • Else, set nested["attr"] to nil
    • Else, create attribute nested and set nested["attr"] to nil.

This should be done during the run step, so that we don't populate empty attributes for plugins that are disabled.

This will help new plugin authors learning how to write plugins. New authors may not have encountered the attribute Mash.new unless attribute pattern and may be confused why their attribute is not being populated correctly.

This will help plugin authors and plugin maintainers ensure compatibility when new plugins providing attribute (with attribute Mash.new) are submitted. Maintainers and authors will not have to modify existing plugins by appending unless attribute.

Changes to the run_plugin dsl method are required. The new behavior should be to query the plugin's provides and initialize attributes in the manner described above before running collect_data. Changes to existing plugins are not immediately required, as attribute Mash.new unless attribute will set attribute to Mash.new when attribute is nil. However, we should update existing plugins as soon as possible to discourage this pattern. Documentation will require updating as it will no longer be necessary to include attribute Mash.new in plugin code.

Thoughts @opscode/client-engineers ?

mcquin avatar Sep 15 '14 21:09 mcquin

Yep this design sounds good, needs implementation.

lamont-granquist avatar Feb 04 '15 18:02 lamont-granquist

:+1:

smurawski avatar Feb 04 '15 18:02 smurawski

+1

charlesjohnson avatar Feb 04 '15 18:02 charlesjohnson

:+1:

danielsdeleo avatar Feb 04 '15 18:02 danielsdeleo

I wrote this before we started the RFC process. Should I put something regarding this there? O/w happy to start working on it. :)

mcquin avatar Feb 04 '15 18:02 mcquin

Doesn't seem particularly large or controversial to me.

danielsdeleo avatar Feb 04 '15 19:02 danielsdeleo

There's a question of how we get this in -- happy to have you do the contribution @mcquin if you have slack, otherwise we're trying to identify a way to slot this to people that have bandwidth and prioritization to do it.

Let us know if you're go though @mcquin, assuming you have the time, and no pressure if you don't. We're in agreement we'd like to get this in eventually.

adamedx avatar Feb 04 '15 19:02 adamedx

I might have a dusty wip branch, but it's nothing special... @adamedx I don't have the spare time to work on this at the moment, so if someone wants to take it sooner I will support them on their venture.

mcquin avatar Feb 13 '15 18:02 mcquin