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

Enhance Generic Object methods. #11876

Merged
merged 4 commits into from Oct 21, 2016
Merged

Conversation

lfu
Copy link
Member

@lfu lfu commented Oct 12, 2016

Add GenericObjectDefinition#find_objects method.
Expose more GenericObjectDefinition methods into automate.

cc @gmcculloug @bzwei @mkanoor

GenericObject.create!({:generic_object_definition => self}.merge(options))
end

def find_objects(options)
Copy link
Contributor

Choose a reason for hiding this comment

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

@lfu can you add comments here what fields are searchable, and possibly what are expected to be searchable in the future?

json_options = dup.extract!(*property_attributes.keys)

unless dup.empty?
err_msg = _("Attributes [%{attrs}] do not exist") % {:attrs => dup.keys.join(", ")}
Copy link
Contributor

Choose a reason for hiding this comment

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

The error message may be misleading since you only support property_attributes for now. When a search for associations for example, the field may exist but simply not searchable.

@lfu lfu force-pushed the enhance_go_methods branch 2 times, most recently from a5d1e2c to 14c17ea Compare October 14, 2016 13:11
# find_objects(:vms => [23, 26])
#
def find_objects(options)
dup = options.stringify_keys!.dup
Copy link
Contributor

Choose a reason for hiding this comment

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

we don't want to modify options, so dup first and then stringify_keys.

json_options = dup.extract!(*(property_attributes.keys + property_associations.keys))

unless dup.empty?
err_msg = _("[%{attrs}]: not defined for Generic Object of %{name}") % {:attrs => dup.keys.join(", "),
Copy link
Contributor

Choose a reason for hiding this comment

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

consider the wording not searchable instead of not defined, for example a method name might be defined but not searchable.

@g1.vms = [vm[0], vm[1], vm[2]]
@g1.save!

@options = {:vms => [vm[0].id, vm[1].id]}
Copy link
Contributor

Choose a reason for hiding this comment

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

I like jsonb that can do this partial matching search 👍

@lfu lfu force-pushed the enhance_go_methods branch 2 times, most recently from ba91a72 to cb20bae Compare October 18, 2016 13:04
# find_objects(:vms => [23, 26])
#
def find_objects(options)
dup = options.dup.stringify_keys!
Copy link
Contributor

Choose a reason for hiding this comment

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

sorry @lfu. dup = options.stringify_keys should be sufficient.

Copy link
Member Author

Choose a reason for hiding this comment

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

@bzwei 👍

# To query based on GenericObject AR attributes and property attributes
# find_objects(:name => "TestLoadBalancer", :uid => '0001', :prop_attr_1 => 10, :prop_attr_2 => true)
#
# To query based on property associations
Copy link
Contributor

Choose a reason for hiding this comment

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

To query based on property associations. The array does not need to be a complete list, but must contain only AR ids.

@miq-bot
Copy link
Member

miq-bot commented Oct 18, 2016

Checked commits lfu/manageiq@048f104~...fde84cb with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1
6 files checked, 3 offenses detected

app/models/generic_object_definition.rb

  • ❕ - Line 58, Col 3 - Metrics/AbcSize - Assignment Branch Condition size for find_objects is too high. [23.37/20]

spec/models/generic_object_spec.rb

args.each_with_index { |item, idx| attrs["param_#{idx + 1}".to_sym] = item }
args.each_with_index do |item, idx|
attrs["param_#{idx + 1}".to_sym] = item
attrs["param_#{idx + 1}_type".to_sym] = item.class.name
Copy link
Contributor

Choose a reason for hiding this comment

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

For container Hash and Array, we can't know the type of each element. This is a limitation but I don't think we need to address it.

Copy link
Member

Choose a reason for hiding this comment

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

Agreed, there are some limitations to calling methods on Generic Objects.

@bzwei
Copy link
Contributor

bzwei commented Oct 19, 2016

LGTM. Thanks.

@lfu
Copy link
Member Author

lfu commented Oct 21, 2016

@gmcculloug Anything to update?

@gmcculloug
Copy link
Member

Looks good. Thanks!

@gmcculloug gmcculloug merged commit fd0a6bf into ManageIQ:master Oct 21, 2016
@gmcculloug gmcculloug added this to the Sprint 48 Ending Oct 24, 2016 milestone Oct 21, 2016
chessbyte pushed a commit that referenced this pull request Oct 22, 2016
Enhance Generic Object methods.
(cherry picked from commit fd0a6bf)
@chessbyte
Copy link
Member

Euwe Backport details:

$ git log -1
commit 3e851efb17b9aa03b4869e53f7fad0529bcf1a3d
Author: Greg McCullough <gmccullo@redhat.com>
Date:   Fri Oct 21 18:15:09 2016 -0400

    Merge pull request #11876 from lfu/enhance_go_methods

    Enhance Generic Object methods.
    (cherry picked from commit fd0a6bfd3ff55d8905c8ea8155607615a6a7286f)

@lfu lfu deleted the enhance_go_methods branch March 2, 2017 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants