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

Policyfile get/set API #111

Merged
merged 10 commits into from
Feb 11, 2015
Merged

Policyfile get/set API #111

merged 10 commits into from
Feb 11, 2015

Conversation

danielsdeleo
Copy link
Contributor

Policyfile Get/Set API

Implements the Policyfile API first iteration, corresponding to chef-boneyard/chef-pedant#90 This API is likely to be revised; An RFC describing the final API will be proposed soon, and chef-zero will be updated accordingly then. In the interim this feature should be safe to merge since it's gated by two feature flags on the client side (and warnings about instability are emitted by both ChefDK and Chef Client when using it).

Future ruby has automagic nested exceptions but with current ruby
raising a new exception during rescue obscures the backtrace
@danielsdeleo
Copy link
Contributor Author

@jkeiser I think this is ready for review and eventual merge now

@danielsdeleo
Copy link
Contributor Author

Note that ChefFS support for policyfile objects is only in master of Chef, but Chef 12.x is on RSpec 3, while pedant is on RSpec 2 (though I recently cleaned up a ton of deprecation warnings there). If we want to have ChefFS support and pedant in the same gemfile, that means pedant will have to get upgraded to RSpec 3.

The memore store and ChefFS store have different behaviors when writing
to an existing object, so work around that by checking for the existence
of the item before creating/updating.
* Chef Client no longer supports Ruby 1.9.3 or 1.8, so drop both of those.
* Drop the patch version number from MRI versions so we just use the
  latest patch
* Turn off sudo. This allows us to run on the faster container-based
  build cluster
@danielsdeleo
Copy link
Contributor Author

@jkeiser Ok, this is ready to go. Important to point out that this drops support for Ruby 1.9.3, though that will be EOL next month, so it shouldn't be a problem (also, it's just removed from the test matrix, not actively broken). That was required in order to bring chef up-to-date, which in turn is required to get any new code from ChefFS to be used in the tests. Also, I updated pedant to use RSpec 3 which means that pedant and chef are only dependency-compatible if you use newer versions of both.

@jkeiser
Copy link
Contributor

jkeiser commented Jan 29, 2015

Will look as soon as I can but I am in the air tomorrow fyi
On Jan 28, 2015 6:31 PM, "Dan DeLeo" notifications@github.com wrote:

@jkeiser https://github.com/jkeiser Ok, this is ready to go. Important
to point out that this drops support for Ruby 1.9.3, though that will be
EOL next month, so it shouldn't be a problem (also, it's just removed from
the test matrix, not actively broken). That was required in order to bring
chef up-to-date, which in turn is required to get any new code from ChefFS
to be used in the tests. Also, I updated pedant to use RSpec 3 which means
that pedant and chef are only dependency-compatible if you use newer
versions of both.


Reply to this email directly or view it on GitHub
#111 (comment).

@@ -112,9 +112,13 @@ def fix_exceptions
begin
yield
rescue DataAlreadyExistsError => e
raise DataAlreadyExistsError.new([ 'organizations', single_org ] + e.path, e)
err = DataAlreadyExistsError.new([ 'organizations', single_org ] + e.path, e)
err.set_backtrace(e.backtrace)
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh wow!!! I didn't know you could do this! Awesome!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ruby's eventually getting nested exceptions which may be slightly better than doing it this way, but this is a good pattern for when you want to change the class and message of some lower level exception.

@jkeiser
Copy link
Contributor

jkeiser commented Feb 11, 2015

👍

@jkeiser jkeiser merged commit fb454e1 into master Feb 11, 2015
@danielsdeleo danielsdeleo deleted the policyfile-get-set-api branch February 12, 2015 02:07
@chef chef locked and limited conversation to collaborators Nov 16, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants