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

Return an empty relation instead of an array from db_relation() #15325

Merged
merged 1 commit into from
Jun 7, 2017

Conversation

mansam
Copy link
Contributor

@mansam mansam commented Jun 6, 2017

db_relation() returns an empty array if it couldn't determine another relation to use. This empty array doesn't work with chained activerecord methods like find_each, and breaks functions that assume they've been given an activerecord relation like populate_db_data_index! This PR fixes the problem by using the inventory collection's model class to return a null relation which behaves correctly with chained activerecord methods.

db_relation() returns an empty array if it couldn't determine
another relation to use. This empty array doesn't work with
chained activerecord methods like find_each, and breaks
functions like populate_db_data_index!
@miq-bot
Copy link
Member

miq-bot commented Jun 6, 2017

Checked commit mansam@03f6941 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
1 file checked, 0 offenses detected
Everything looks fine. 👍

Copy link
Member

@durandom durandom left a comment

Choose a reason for hiding this comment

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

👍 from me

@Ladas ?

@Ladas
Copy link
Contributor

Ladas commented Jun 7, 2017

nice catch, looks great 👍

@Ladas
Copy link
Contributor

Ladas commented Jun 7, 2017

@mansam btw. I've been trying to quickly skip processing of the IC if it would not do anything, by checking inventory_collection.noop? . I am using that in the AWS Targeted Refresh

@durandom
Copy link
Member

durandom commented Jun 7, 2017

@miq-bot assign @agrare

@miq-bot miq-bot assigned agrare and unassigned durandom Jun 7, 2017
@agrare agrare added this to the Sprint 63 Ending Jun 19, 2017 milestone Jun 7, 2017
@agrare agrare merged commit 2653cb2 into ManageIQ:master Jun 7, 2017
@mansam
Copy link
Contributor Author

mansam commented Jun 7, 2017

@Ladas Cool, I'll have to check out the new stuff you've been doing in the Amazon repo.

@Ladas
Copy link
Contributor

Ladas commented Jun 8, 2017

@mansam the targeted refresh is more integrated to the InventoryCollection now, so soon we should be able to move to the class level definition like we have in Ansible.

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.

6 participants