Native Chef Helpers functions duplicated as node methods

[facebook]

Native Chef Helpers functions duplicated as node methods

[nmcspadden]

The darwin? one, for example, was added in 15.5: https://github.com/chef/chef/blob/main/chef-utils/lib/chef-utils/dsl/os.rb#L47 These predate nearly all of the built-in helper functions. We've just never migrated because it didn't make a difference.

…

On Thu, Nov 4, 2021 at 7:35 AM Austin Culter ***@***.***> wrote: I don't understand why some of the native Chef helper functions [1] -- like def linux?, def macos?, def debian_family?, et al -- are being duplicated in fb_helpers, cpe_helpers. Rather than extending the node object to contain functions already available to the Chef DSL, why not just use the built-in ones? Frame number: 0/26 From: /etc/chef/local-mode-cache/cache/cookbooks/cpe_my_org_init/recipes/default.rb:17 Chef::Mixin::FromFile#from_file: 12: # of patent rights can be found in the PATENTS file in the same directory. 13: # 14: require 'pry' 15: binding.pry 16: # Start an empty run_list so we can append to it => 17: run_list = [] 18: 19: # All machines 20: run_list += [ 21: 'global_base', 22: 'my_org_base', [1] pry(#<Chef::Recipe>)> show-method linux? From: /opt/chef/embedded/lib/ruby/gems/3.0.0/gems/chef-utils-17.7.29/lib/chef-utils/dsl/os.rb:28: Owner: ChefUtils::DSL::OS Visibility: public Signature: linux?(node=?) Number of lines: 3 def linux?(node = __getnode) node["os"] == "linux" end [2] pry(#<Chef::Recipe>)> show-method node.linux? From: /etc/chef/local-mode-cache/cache/cookbooks/fb_helpers/libraries/node_methods.rb:50: Owner: Chef::Node Visibility: public Signature: linux?() Number of lines: 3 def linux? self['os'] == 'linux' end *Can anyone explain why this is necessary/beneficial?* [1] See: https://github.com/chef/chef/blob/main/chef-utils/lib/chef-utils/dsl/os.rb & https://github.com/chef/chef/blob/main/chef-utils/lib/chef-utils/dsl/platform.rb & https://github.com/chef/chef/blob/main/chef-utils/lib/chef-utils/dsl/platform_family.rb — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#265>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJFTX6VT4DYQ4LGDMT56MLUKKKZ7ANCNFSM5HLS6RBA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

-- -- Nick McSpadden ***@***.***

[ChefAustin]

Would you entertain a PR to remove them? Or is your fleet of devices running pre-15.5 client versions and such a PR would break them?

[nmcspadden]

We likely wouldn't accept that PR without pretty thorough testing in our environment. I'd encourage you to put one up to kickstart the conversation, but migrating that is also pretty low-pri unless there's a demonstrable difference in performance/time - or some other indication of problem that this is causing.

…

On Thu, Nov 4, 2021 at 8:51 AM Austin Culter ***@***.***> wrote: Would you entertain a PR to remove them? Or is your fleet of devices running pre-15.5 client versions and such a PR would break them? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#265 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJFTX3H6EKZXKBSUL5TRLDUKKTZVANCNFSM5HLS6RBA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

-- -- Nick McSpadden ***@***.***

[ChefAustin]

We likely wouldn't accept that PR without pretty thorough testing in our environment. I'd encourage you to put one up to kickstart the conversation, but migrating that is also pretty low-pri unless there's a demonstrable difference in performance/time - or some other indication of problem that this is causing.

PR opened. Let me know if you have any concerns or discover any issues when you get around to testing internally.

[erikng]

Yesterday I discovered namespace collisions breaking a lot of our linux logic. facebook/chef-cookbooks#211