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

Bulk updates requires a lot of studdering #14

Open
TomOnTime opened this issue May 27, 2015 · 6 comments
Open

Bulk updates requires a lot of studdering #14

TomOnTime opened this issue May 27, 2015 · 6 comments
Labels
enhancement New feature or request

Comments

@TomOnTime
Copy link

TomOnTime commented May 27, 2015

Problem: If one has to set a lot of variables in many files (for example, using each()), you have to repeat yourself a lot.

For example, this is the cleanest I've been able to write for setting many variables.

  Shellvar {
    ensure => present,
    target => $ifile,
  }

  shellvar { "${name} DEVICE": variable => 'DEVICE', value  => $name}
  shellvar { "${name} NAME": variable => 'NAME', value  => $name}
  shellvar { "${name} HWADDR": variable => 'HWADDR', value => $hwaddress}
  shellvar { "${name} TEAM_MASTER": variable => 'TEAM_MASTER', value => $team_master}
  shellvar { "${name} DEVICETYPE": variable => 'DEVICETYPE', value  => 'TeamPort'}
  shellvar { "${name} ONBOOT": variable => "TYPE", value => 'yes'}
  shellvar { "${name} TYPE": variable => 'TYPE', value => 'Ethernet'}
  shellvar { "${name} NM_CONTROLLED": value => 'NM_CONTROLLED', variable => 'no'}

There are a few problems with this:

  • The namevar repeats the variable name, which is error prone. They may get out of sync.
  • Specifying variable => and value => again and again is very verbose. It hides meaning.
  • It is slow. Each one of these will require starting up augeas, parsing the file, etc.
  • You didn't notice that "ONBOOT" sets the wrong variable, or that NM_CONTROLLED reversed the value and the variable.

Proposed Solution: For mass updates, it would be more natural to have a function that takes a filename and a list of settings:

  shellvar_bulk( $ifile, [
          "DEVICE=$name", 
          "NAME=$name", 
          "HWADDR=$hwaddress", 
          "TEAM_MASTER=$TEAM_MASTER",
          'DEVICETYPE=TeamPort',
          'ONBOOT=yes',
          'TYPE=Ethernet',
          'NM_CONTROLLED=no',
     ])
@raphink
Copy link
Member

raphink commented May 27, 2015

That could be interesting indeed. Do you want to provide a PR with such a shellvar::bulk wrapping definition?

@TomOnTime
Copy link
Author

I would but... sadly I don't think I know enough Ruby to add a function that does that. If I did it as a define() then it wouldn't have the performance benefit.

@raphink
Copy link
Member

raphink commented May 27, 2015

I was actually thinking of a DSL define. How would you expect to see performance benefit?

@raphink
Copy link
Member

raphink commented May 27, 2015

On the performance issue: since puppet 3.4, augeasproviders use a shared Augeas handler for all resources. Hence, Augeas is only started once, and only parses target files twice as most (one read to fill the tree, a second read to calculate the diff if changes were made, and one write to flush changes if any). All you need is to use puppet 3.4+. The only overhead to using separate resources currently is stuff done entirely in memory, so the gain wouldn't be great.

@TomOnTime
Copy link
Author

Oh, so the optimization is already in there. That's awesome!

I'll see if I can work up a DSL define.

@raphink
Copy link
Member

raphink commented May 29, 2015

Actually, the lib still saves the Augeas tree once per modified resource (vs once per modified property). I don't think it would be good to change this though, as that would mean that if a resource produces an invalid tree, all resources of that file will fail to apply.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants