Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add checks to resource_type handler and code objects #232

Merged
merged 1 commit into from
Jun 1, 2020

Conversation

scotje
Copy link
Contributor

@scotje scotje commented May 1, 2020

Resolves #229

@scotje
Copy link
Contributor Author

scotje commented May 1, 2020

cc @jbondpdx @eputnam

@scotje scotje force-pushed the 229_add_checks_to_resource_types branch from c5593a0 to 926af93 Compare May 29, 2020 23:10
@scotje scotje marked this pull request as ready for review May 29, 2020 23:36
@scotje scotje requested review from eputnam and a team May 29, 2020 23:36
@registry[:checks].sort_by { |p| p[:name] }
end

# "checks" (such as "onlyif" or "creates") are another type of property
Copy link
Contributor

Choose a reason for hiding this comment

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

is new check really a totally new type of property invented only for the exec type?!

https://github.com/puppetlabs/puppet/search?q=newcheck&unscoped_q=newcheck

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤷‍♂️

Is it also available for custom resource types and providers?

Copy link
Contributor

Choose a reason for hiding this comment

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

Nope. It's defined and used in only that one type.

@joshcooper is newcheck something that we might want to elevate to be part of the actual T&P API? Or could we refactor these psuedoparameters into actual params? My concern is that with this being special-cased is that other parts of the ecosystem will almost certainly not account for these either.

(We'll obviously still have to handle this in strings for backwards compatibility)

Copy link
Contributor

@binford2k binford2k Jun 1, 2020

Choose a reason for hiding this comment

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

Or maybe it doesn't matter. It's clearly also defining parameters and strings just doesn't catch it because it's static analysis. The type object will have a param defined.

https://github.com/puppetlabs/puppet/blob/b6d0627415fcf8d25c81e044acf95faf00880a22/lib/puppet/type/exec.rb#L70-L75

Copy link
Contributor

Choose a reason for hiding this comment

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

newcheck is just for the exec type, however, the net result is :creates, etc are metaprogrammed parameters. I'm not familiar with how strings introspects the type/property/parameter, but it's possible to access the documentation for all exec parameters (which include properties):

irb(main):001:0> require 'puppet'
=> true
irb(main):002:0> Puppet::Type.type(:exec)
=> Puppet::Type::Exec
irb(main):003:0> Puppet::Type.type(:exec).parameters
=> [:command, :path, :user, :group, :cwd, :logoutput, :refresh, :environment, :umask, :timeout, :tries, :try_sleep, :refreshonly, :creates, :unless, :onlyif, :provider]
irb(main):004:0> Puppet::Type.type(:exec).paramclass(:creates)
=> Puppet::Type::Exec::ParameterCreates
irb(main):005:0> Puppet::Type.type(:exec).paramclass(:creates).doc
=> "A file to look for before running the command. The command will\nonly run if the file **doesn't exist.**\n\nThis parameter doesn't cause Puppet to create a file; it is only\nuseful if **the command itself** creates a file.\n\n    exec { 'tar -xf /Volumes/nfs02/important.tar':\n      cwd     => '/var/tmp',\n      creates => '/var/tmp/myfile',\n      path    => ['/usr/bin', '/usr/sbin',],\n    }\n\nIn this example, `myfile` is assumed to be a file inside\n`important.tar`. If it is ever deleted, the exec will bring it\nback by re-extracting the tarball. If `important.tar` does **not**\nactually contain `myfile`, the exec will keep running every time\nPuppet runs.\n\n"

@scotje
Copy link
Contributor Author

scotje commented Jun 1, 2020

Based on how strings analyzes things, I think the current approach is still the best at the moment, so I'm going to merge this. Future PRs to improve parsing welcome. :)

@scotje scotje merged commit 3ed398f into puppetlabs:master Jun 1, 2020
@scotje scotje deleted the 229_add_checks_to_resource_types branch June 1, 2020 20:01
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

exec type in generated docs missing attributes creates, onlyif
3 participants