-
Notifications
You must be signed in to change notification settings - Fork 42
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
(PDK-512) Add support for simple_get_filter #37
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.
One mismatch with the spec, and two minor cleanups.
🎉
@@ -3,9 +3,9 @@ | |||
|
|||
RSpec.describe Puppet::ResourceApi::SimpleProvider do | |||
let(:context) { instance_double('Puppet::ResourceApi::BaseContext', 'context') } | |||
let(:provider_class) do | |||
let(:provider_class) do # here... |
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.
there?
lib/puppet/resource_api.rb
Outdated
unknown_features = definition[:features] - supported_features | ||
Puppet.warning("Unknown feature detected: #{unknown_features.inspect}") unless unknown_features.empty? | ||
|
||
|
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.
whitespace should be fixed in previous commit
@@ -7,7 +7,12 @@ module Puppet::ResourceApi | |||
class SimpleProvider | |||
def set(context, changes) | |||
changes.each do |name, change| | |||
is = change.key?(:is) ? change[:is] : (get(context) || []).find { |r| r[:name] == name } | |||
is = if context.feature_support?('simple_get_filter') | |||
change.key?(:is) ? change[:is] : (get(context, [name]) || []).first |
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.
according to the spec, get(context, [name])
is allowed to return more than one resource, and the first is not necessarily the one we were asking for. This still needs the same .find { |r| r[:name] == name }
as below.
No description provided.