-
Notifications
You must be signed in to change notification settings - Fork 261
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
[chef][windows] allow custom prefix when specifying agent artifact+ve… #634
Conversation
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.
A few comments, overall LGTM!
Of course, a review from @remeh will be necessary as well :)
Also, let's mention in the PR description the use cases for this. In particular, a user installing released Agents should never have to specify this new attribute
metadata.rb
Outdated
@@ -4,7 +4,7 @@ | |||
license 'Apache-2.0' | |||
description 'Installs/Configures datadog components' | |||
long_description IO.read(File.join(File.dirname(__FILE__), 'README.md')) | |||
version '3.1.0' | |||
version '3.1.1' |
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.
version bump should be removed from PR
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.
Yeah, we're using this from the QA env, so I needed the bump. But we'll remove that before merging.
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.
Let's remove this and merge to master
!
recipes/_install-windows.rb
Outdated
@@ -40,8 +40,11 @@ | |||
dd_agent_latest = 'ddagent-cli-latest' | |||
end | |||
|
|||
# If a different file prefix is specified, use that: | |||
dd_agent_installer_prefix = node['datadog']['windows_installer_prefix'] ? node['datadog']['windows_installer_prefix'] : 'ddagent-cli' |
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.
The attribute windows_installer_prefix
must be added to attributes/default.rb
, and documented there.
Also, nit: for consistency with other Windows-related attributes, I slightly prefer a name such as windows_agent_installer_prefix
, even though it's longer
Co-Authored-By: Olivier Vielpeau <[email protected]>
…adog into jaime/win_custom_prefix
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.
Great addition, thank you!
metadata.rb
Outdated
@@ -4,7 +4,7 @@ | |||
license 'Apache-2.0' | |||
description 'Installs/Configures datadog components' | |||
long_description IO.read(File.join(File.dirname(__FILE__), 'README.md')) | |||
version '3.1.0' | |||
version '3.1.1' |
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.
Let's remove this and merge to master
!
…rsion