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
Merged

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

merged 1 commit into from
Feb 13, 2017

Conversation

bai
Copy link
Contributor

@bai 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
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
Copy link
Contributor

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
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
Copy link
Contributor Author

bai commented Feb 13, 2017

Friendly bump 💯

cc: @olivielpeau

Copy link
Member

@degemer degemer left a comment

Choose a reason for hiding this comment

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

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
@bai bai deleted the apt-transport-https branch February 13, 2017 16:32
@olivielpeau olivielpeau added this to the 2.9.0 milestone Feb 13, 2017
@andrewjamesbrown
Copy link

@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
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants