Should we remove all items under the 'MODULE_DATA' epic?
Add your comments directly to the page. Include links to any relevant research, data, or feedback.
Status | DECIDED |
|---|---|
Impact | LOW |
Driver | @Trevor Vaughan |
Approver |
|
Contributors | @Liz Nemsick @Dylan Cochran (Unlicensed) @Brandon Riden (Unlicensed) @Kendall Moore @Nick Markowski @Nick Miller (Unlicensed) @Judith Johnson @Shark Bruhaha (Unlicensed) @Jeanne Greulich @Michael Morrone @Chris Tessmer |
Informed |
|
Due date | Feb 8, 2019 |
Outcome | Option 2 |
Background
The MODULE-DATA epic is outdated and should not be done arbitrarily.
In general, the puppet community suggests using module data as appropriate and `params.pp` as appropriate.
Also, there has been no fix for the fact that even default data with no logic paths cannot be handled by `puppet strings` which makes the migration of all default data problematic for auto-generating documentation that is actually useful to end users.
Relevant data
Of the items in the table below, I believe that all can be closed except for SIMP-3281 which seems like a useful linting addition (but may already be completed)
Options considered
| Option 1: | Option 2: |
|---|---|---|
Description | Remove all open items besides SIMP-3218 | Retain all open items and triage individually |
Pros and cons | Allows natural migration to module data as appropriate None | Allows for full analysis of each remaining module Will probably camp on the backlog without resolution as it already has |
Estimated cost | LOW | MEDIUM |
Votes |
| 5 |
Voters |
| @Kendall Moore @Dylan Cochran (Unlicensed) @Chris Tessmer @Judith Johnson @Trevor Vaughan |
Action items
Outcome
Comments
I understand the reason to move params.pp logic to data in modules but is it necessary to move all settings to another place outside of the module. A simple
default setting that doesn't change for OS (or something easily defined in the data directory) is better left in the code. (The ntp module example in the tickets moves all settings to the data directory putting common data in the common.yaml file.) This can make the code harder to follow /read. There are many settings which don't rely on logic to be set and seeing them in the module is helpful. Does it benefit the code or performance to move it to data? I think it benefits ease of use and understanding to leave it in the manifest.
I definitely wouldn't think there is a performance increase to moving parameter defaults to data. It's my opinion that having all of the data inside of a data directory is no more or less confusing that having bits of data inside of every manifest in the manifests directory while also having some in the data directory. Having a value defined right in front of you inside of a manifest is helpful to a developer, but it's really not that much of a burden to an end-user to have it defined in data instead. Most end-users are taught to search for a module on the Forge, determine it's value based on the score and README, and then download it into a test environment to ensure it does what they need. If it doesn't they are taught to change values inside of a profile or inside of Hiera. At that point they only need to know the name of the parameter, not it's default value. There are very few cases where a user is going to be scouring the module for all of it's default values before using it so I don't think the scenario of needing to see all of the defaults will really be common.
If by defaults you mean the program's defaults, then yes leave those in the manifest, but I would say that any modification to those (even if it is our default suggested/recommended setting) should be in the data dir and the users can go there to modify/add setting overrides.
I think it would be unwise to remove these items from the backlog and close/dismiss the epic. Using in-module data to define default parameter values for init.pp has been the standard for going on four years now with no sign of that changing. In fact, all of the officially published information on this topic from Puppet would agree with this, including articles on how to migrate from params.pp to a Hieradata approach (see: https://puppet.com/docs/puppet/5.1/hiera_migrate_modules.html). Certainly I can't speak for every voice in the community on their opinions, but there does not exist any modern publication from Puppet that the params pattern is anything short of a hack and there are no instances where it's the recommended approach. It therefore does not make sense for a Puppet-centric product such as SIMP to simply ignore all of the published architecture standards for modules and module data.
The issue mentioned regarding puppet-strings is both relevant and a factor to be weighed here, but not a heavy enough issue that warrants deviating from known correct architecture decisions for modules. The issue doesn't prevent documentation from being built, it simply doesn't allow for the inclusion of the default value of parameters as defined in the module data. That's inconvenient, but not as inconvenient as maintaining legacy code and deviating from what the majority of the Puppet community is doing.