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

Allow for nulls in the exists method #571

Merged
merged 1 commit into from Sep 3, 2019

Conversation

syncrou
Copy link
Contributor

@syncrou syncrou commented Aug 30, 2019

If the allow_null argument is true we now allow any result
to return as true. This nullifies the bool() check
So seems bad, but in some cases when an attribute exists
When looking up 'get_object_attribute_names' we want
the value of that attribute even if it is null
In the case of an allow_null then only a KeyError
will return False

BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1730311

If the allow_null argument is true we now allow any result
to return as true. This nullifies the bool() check
So seems bad, but in some cases when an attribute exists
When looking up 'get_object_attribute_names' we want
the value of that attribute even if it is null
In the case of an allow_null then only a KeyError
will return False
@syncrou
Copy link
Contributor Author

syncrou commented Aug 30, 2019

@miq-bot assign @tinaafitz

cc - @mkanoor

@syncrou
Copy link
Contributor Author

syncrou commented Aug 30, 2019

@miq-bot add_label bug

@miq-bot
Copy link
Member

miq-bot commented Aug 30, 2019

Checked commit syncrou@b1e6c8d with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
0 files checked, 0 offenses detected
Everything looks fine. ⭐

@coveralls
Copy link

coveralls commented Aug 30, 2019

Pull Request Test Coverage Report for Build 3523

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.02%) to 96.9%

Totals Coverage Status
Change from base Build 3515: 0.02%
Covered Lines: 2970
Relevant Lines: 3065

💛 - Coveralls

@mkanoor mkanoor merged commit 2c6c747 into ManageIQ:master Sep 3, 2019
@mkanoor mkanoor added this to the Sprint 120 Ending Sep 16, 2019 milestone Sep 3, 2019
simaishi pushed a commit that referenced this pull request Sep 3, 2019
@simaishi
Copy link
Contributor

simaishi commented Sep 3, 2019

Ivanchuk backport details:

$ git log -1
commit 62108090eb401134c86ba9efa28795e5f710a374
Author: Madhu Kanoor <mkanoor@redhat.com>
Date:   Tue Sep 3 12:00:18 2019 -0400

    Merge pull request #571 from syncrou/allow_nulls_in_exist
    
    Allow for nulls in the exists method
    
    (cherry picked from commit 2c6c747a81a15cb7b9667644dcb4abff91d7a7c1)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1730311

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

6 participants