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

Switch to call lookup_by_* methods #584

Merged
merged 1 commit into from
Oct 7, 2019
Merged

Conversation

astrozzc
Copy link
Contributor

The find_by_* methods defined in manageiq repo has conflicts with
rails dynamic finding and now they have been changed to lookup_by_.
Switching the callers in this repo to the lookup_by_
methods.

Depend on ManageIQ/manageiq#19313, ManageIQ/manageiq-automation_engine#373

The find_by_* methods defined in manageiq repo has conflicts with
rails dynamic finding and now they have been changed to lookup_by_*.
Switching the callers in this repo to the lookup_by_* methods.

Depend on ManageIQ/manageiq#19313
@miq-bot
Copy link
Member

miq-bot commented Sep 24, 2019

Checked commit astrozzc@b5d6dd5 with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🍪

@coveralls
Copy link

coveralls commented Sep 24, 2019

Pull Request Test Coverage Report for Build 3638

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.1%) to 96.657%

Totals Coverage Status
Change from base Build 3612: -0.1%
Covered Lines: 13617
Relevant Lines: 14088

💛 - Coveralls

@@ -19,7 +19,7 @@ def main
category_name = @handle.root['dialog_tag_category']
if category_name.present?
@handle.log(:info, "Selected tag category: #{category_name}")
category = @handle.vmdb(:classification).find_by_name(category_name)
category = @handle.vmdb(:classification).lookup_by_name(category_name)
Copy link
Member

Choose a reason for hiding this comment

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

@mkanoor @tinaafitz The find_by_* calls work today because the automate engine allows these specific calls to pass through, otherwise they need to be specified with expose or similar technique in the service_models.

See https://github.com/lfu/manageiq-automation_engine/blob/master/lib/miq_automation_engine/engine/miq_ae_method_service/miq_ae_service_model_base.rb#L27-L30

Should it now include lookup_by and the depreciation side will require some thinking. Not sure it should hold up these PRs.

Copy link
Contributor

Choose a reason for hiding this comment

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

@gmcculloug This is ok since its our automate methods, we just have to aware of customers scripts and if they use the find_by_* methods.

@gmcculloug gmcculloug self-assigned this Oct 1, 2019
@astrozzc astrozzc marked this pull request as ready for review October 1, 2019 14:24
@gmcculloug gmcculloug merged commit 3d5f5a9 into ManageIQ:master Oct 7, 2019
@gmcculloug gmcculloug added this to the Sprint 122 Ending Oct 14, 2019 milestone Oct 7, 2019
@astrozzc astrozzc deleted the find_by branch October 7, 2019 18:03
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.

5 participants