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

Deprecate non-standard product names. #91

Merged
merged 1 commit into from Mar 30, 2016
Merged

Deprecate non-standard product names. #91

merged 1 commit into from Mar 30, 2016

Conversation

@sersut
Copy link
Contributor

sersut commented Mar 29, 2016

This PR deprecates chef-ha, chef-marketplace, chef-sync, push-client, push-server in favor of ha, marketplace, sync, push-jobs-client, push-jobs-server.

As we are moving to bintray, we have standardized these names in mixlib-install with https://github.com/chef/mixlib-install/pull/77/files . This PR makes sure that backwards compatibility is not broken for changed products.

Fixes #89.

Note that 0.8.0.alpha.8 is the first version of mixlib-install where these new naming is introduced.

/cc: @chef-cookbooks/engineering-services

@scotthain

This comment has been minimized.

Copy link
Contributor

scotthain commented Mar 29, 2016

👍

# Checks the deprecated properties of chef-ingredient and prints warning
# messages if any of them are being used.
#
def check_deprecated_properties

This comment has been minimized.

Copy link
@schisamo

schisamo Mar 29, 2016

Contributor

So the basic rule here is: Dropping the leading chef-* or opscode-* unless the product name is chef-server or chef-backend. We can express that as:

def check_deprecated_properties
  if !(%w(chef-backend chef-server chef-server-ha-provisioning).include?(new_resource.product_name)) && 
      (match = new_resource.product_name.match(/(chef-|opscode-)(?<product_key>.*)/))

    new_product_key = match[:product_key]
    Chef::Log.warn "product_name '#{new_resource.product_name}' is deprecated and it will be removed in the future versions of chef-ingredient. Use '#{new_product_key}' instead of '#{new_resource.product_name}'."
    new_resource.product_name(new_product_key)
  end
end

We can actually greatly simplify this method (and save future additions to the deprecated_product_names hash).

This comment has been minimized.

Copy link
@sersut

sersut Mar 29, 2016

Author Contributor

I would like to keep this minimal by reducing to exactly what has changed.

This comment has been minimized.

Copy link
@schisamo

schisamo Mar 29, 2016

Contributor

What would you like to happen if a user uses a chef-* or opscode-* prefixed product name? Without some logic like I mentioned above their CCR will just fail with an unfriendly message right? At the very least you need to validate what they provide against PRODUCT_MATRIX.products to ensure a valid key has been given. A friendlier approach would removing the prefix and printing the warning.

This comment has been minimized.

Copy link
@sersut

sersut Mar 29, 2016

Author Contributor

I think it would be a good idea to add some validation against PRODUCT_MATRIX. We can add that with a separate PR.

With the above function is everything being handled?

  • 'push-client' => 'push-jobs-client',
  • 'push-server' => 'push-jobs-server'
  • chef-server-ha-provisioning

This comment has been minimized.

Copy link
@schisamo

schisamo Mar 30, 2016

Contributor

I updated the above function to include chef-server-ha-provisioning. I think we'll need a separate if check for the old push-client and push-server naming. Good times.

This comment has been minimized.

Copy link
@sersut

sersut Mar 30, 2016

Author Contributor

Should be fixed now.

@sersut sersut force-pushed the sersut/deprec branch from 31e72cb to 8c2a60e Mar 29, 2016
@sersut sersut force-pushed the sersut/deprec branch from 8c2a60e to 60875d1 Mar 30, 2016
…ver in favor of ha, marketplace, sync, push-jobs-client, push-jobs-server.
@schisamo

This comment has been minimized.

Copy link
Contributor

schisamo commented Mar 30, 2016

👍

@sersut sersut merged commit 8e3f52f into master Mar 30, 2016
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@sersut sersut deleted the sersut/deprec branch Mar 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.