Skip to content
This repository has been archived by the owner on Nov 1, 2023. It is now read-only.

fix bool queries #597

Merged
3 commits merged into from
Feb 26, 2021
Merged

fix bool queries #597

3 commits merged into from
Feb 26, 2021

Conversation

bmc-msft
Copy link
Contributor

This addresses broken queries used for identifying outdated nodes.

@bmc-msft bmc-msft changed the title fix bool queries (used for marking nodes as outdated) fix bool queries Feb 25, 2021
parts.append("%s eq %s" % (field_name, str(x).lower))
if isinstance(values[0], int):
parts.append("%s eq %s" % (field_name, str(x).lower()))
elif isinstance(values[0], int):
Copy link
Member

Choose a reason for hiding this comment

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

Confirming: this shouldn't actually matter, since bool is not a subclass of int. You are just updating this to be consistent with the larger control flow here, which is conceptually a big match.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately, bool is a subclass of int. This was generating queries that were field == 0, where we needed field eq false

Python 3.8.5 (default, Jan 27 2021, 15:41:15)
[GCC 9.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> a = False
>>> isinstance(a, int)
True
>>>

@@ -148,8 +148,8 @@ def build_filters(
for x in values:
if not isinstance(x, bool):
raise TypeError("unexpected type")
parts.append("%s eq %s" % (field_name, str(x).lower))
if isinstance(values[0], int):
parts.append("%s eq %s" % (field_name, str(x).lower()))
Copy link
Member

Choose a reason for hiding this comment

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

This is where the actual bug was.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a two-part bug.

@ghost
Copy link

ghost commented Feb 26, 2021

Hello @bmc-msft!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit 8600a44 into microsoft:main Feb 26, 2021
@bmc-msft bmc-msft deleted the fix-bool-query branch March 19, 2021 15:41
@ghost ghost locked as resolved and limited conversation to collaborators Apr 18, 2021
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants