-
Notifications
You must be signed in to change notification settings - Fork 2
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
Update custom provider to return valid package resource info #7
base: master
Are you sure you want to change the base?
Conversation
Fix puppet package resource and return valid data from the te_agent_bin provider.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need to close your pull request and re-submit. You can just push additional commits to the same branch to resolve any issues.
@@ -14,16 +14,41 @@ | |||
has_feature :versionable | |||
has_feature :install_options | |||
|
|||
confine :osfamily => [ :RedHat ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This provider is used for all non-Windows operating systems.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed confines
@@ -14,16 +14,41 @@ | |||
has_feature :versionable | |||
has_feature :install_options | |||
|
|||
confine :osfamily => [ :RedHat ] | |||
confine :exists => "/etc/init.d/twdaemon" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't this prevent the provider from working if the agent isn't already installed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed confines
def self.instances | ||
paths = [] | ||
versions = [] | ||
File.open('/etc/init.d/twdaemon').each_line do |r| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will error out if the agent isn't installed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed with latest commit : sudodevnull@355c65b
def self.prefetch(packages) | ||
packages.each do |name, pkg| | ||
version = get_version(pkg) | ||
pkg.provider = new({:ensure => version, :name => name, :provider => :te_agent_bin}) if version | ||
pkg.provider = new({:name => name, :ensure => version, :provider => :te_agent_bin}) if version |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this changing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
standardize name before version
def query | ||
version = get_version | ||
version ? {:ensure => version, :name => resource[:name], :provider => :te_agent_bin} : nil | ||
version ? {:name => resource[:name], :ensure => version, :provider => :te_agent_bin} : nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this changing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the confines and unnecessary spacing
@@ -82,7 +107,6 @@ def install_options | |||
|
|||
def join_options(options) | |||
return unless options | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this changing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove unnecessary spacing
One of your commit comments should also reference the issue that it's resolving. |
My commit references issue #8 at the end. |
What are the changes still being requested here? |
There are still open questions in the discussion of issue #8 |
I updated the provider to return valid data and made a few other adjustments. The self.instances validates version and installation by starting in the /etc/init.d/twdaemon file where it grabs the working dir, modifies it to pinpoint the version file, and pull the version from the version file. It also returns the name of the package resource as it is defined in the module. This is to resolve Issue 8.