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

Rename package[apt-transport-https] resource for Chef 13 compatibility #388

Merged
merged 1 commit into from Feb 13, 2017

Conversation

Projects
None yet
5 participants
@bai
Copy link
Contributor

bai commented Jan 3, 2017

This PR renames package[apt-transport-https] to avoid resource cloning.

Currently, when DataDog cookbook used with apt cookbook 2.9.1 or later (released October 24, 2015), Chef throws a deprecation warning:

Deprecated features used!
  Cloning resource attributes for apt_package[apt-transport-https] from prior resource
Previous apt_package[apt-transport-https]: /var/chef/cache/cookbooks/apt/recipes/default.rb:85:in `from_file'
Current  apt_package[apt-transport-https]: /var/chef/cache/cookbooks/datadog/recipes/repository.rb:24:in `from_file' at 1 location:
    - /var/chef/cache/cookbooks/datadog/recipes/repository.rb:24:in `from_file'
   See https://docs.chef.io/deprecations_resource_cloning.html for further details.

Originally this PR suggested to just remove the resource and bump apt cookbook dependency but it's been decided to rename the resource instead — for those who depend on an older apt cookbook elsewhere.

@degemer

This comment has been minimized.

Copy link
Member

degemer commented Jan 3, 2017

Hello @bai, thank you for your contribution and explanation !

We try to avoid as much as possible setting requirements for our dependencies if there is another way around (even if the requirement is loose), and I think there is be in this case: renaming the resource. If I'm right, this would allow us to maintain our current level of compatibility with the apt cookbook while being compatible with Chef 13.

On a side note, do you know when Chef 13 will be released ? I couldn't find an official release date.

In the long term, we will definitely use your solution, but we know that for now some of our customers are still using an older apt version, and don't want to force them to upgrade.

Let me know what you think or our proposed solution, or if you have any question!

@degemer degemer self-assigned this Jan 3, 2017

@martinisoft

This comment has been minimized.

Copy link
Contributor

martinisoft commented Jan 7, 2017

On a side note, do you know when Chef 13 will be released ? I couldn't find an official release date.

This will happen April 2017 per this accepted RFC for release cadence

@bai

This comment has been minimized.

Copy link
Contributor Author

bai commented Feb 1, 2017

Apologies with getting this one forgotten, I've pushed the fix. 👀

EDIT: Updated title/description.

@bai bai changed the title Slightly bump apt cookbook to improve Chef 13 compatibility Rename package[apt-transport-https] resource for Chef 13 compatibility Feb 1, 2017

@bai

This comment has been minimized.

Copy link
Contributor Author

bai commented Feb 13, 2017

Friendly bump 💯

cc: @olivielpeau

@degemer
Copy link
Member

degemer left a comment

Sorry, I missed your push & previous comment. 😞
Thank you for making this change!

It means we'll have to release a new cookbook version for Chef 13 @olivielpeau (hopefully this is the only needed change) - thank you for linking this RFC @martinisoft, I didn't know about it.

@degemer degemer merged commit 151e416 into DataDog:master Feb 13, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@bai bai deleted the bai:apt-transport-https branch Feb 13, 2017

@olivielpeau olivielpeau added this to the 2.9.0 milestone Feb 13, 2017

@andrewjamesbrown

This comment has been minimized.

Copy link

andrewjamesbrown commented Feb 16, 2017

@degemer I noticed that there are also Chef13 deprecation warnings/errors related to https://github.com/DataDog/chef-datadog/blob/master/recipes/dd-handler.rb#L90

See https://github.com/chef-cookbooks/chef_handler/issues/45 for more details

@degemer

This comment has been minimized.

Copy link
Member

degemer commented Feb 21, 2017

Thanks for reporting this @andrewjamesbrown !

Do you have the same issue with chef_handler 2.x ? We haven't had a chance to test it yet, but if it fixes this issue and the behaviour stays the same, we could relax this requirement: https://github.com/DataDog/chef-datadog/blob/master/metadata.rb#L25

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.