Hints with values of None seem to be broken

I attempted to patch Hints using something like:

http://cdn.pasteraw.com/ftlwrezifkpzaira0xgevf0nshx1c9a

But no luck

Think we fall in this issue: http://stackoverflow.com/questions/5602918/postgresql-select-null-values-in-sqlalchemy

Our Hints() should add a "... is None" to the filter (maybe at [1]?) whenever a None value is passed.

[1] https://github.com/openstack/keystone/blob/0b4f6ebdcc866388e1c6788f45f270414b45aeef/keystone/common/sql/core.py#L337

I have tried to test the scenario using the attached test case script.
And it is working fine with "None" value for "hints".

Nitin,

Thanks for the feedback! Would you be interested in proposing your test case for review at https://review.openstack.org/ ?

Sure. I will raise the review request.

Thanks.

Hi Lance,

I have raised the review request, link is as follows:

   https://review.openstack.org/388541

Could you please add the reviewers, if needed.

Thanks & Regards,
Nitin

Reviewed: https://review.openstack.org/388541
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=c5bcc34c945729469fdc848152964bb2de241899
Submitter: Jenkins
Branch: master

commit c5bcc34c945729469fdc848152964bb2de241899
Author: nitin-29-gupta <email address hidden>
Date: Wed Oct 19 13:32:27 2016 +0530

    Add test cases for passing "None" as a hint

    Adds tests for the use of hints in the credentials API.

    Related-Bug: 1614154
    Change-Id: I2e6fde1696e44f1b9456737f7c643e757cd3b758

Hi Lance,

Patch has been merged into master branch.
Could Please check and confirm.
Also, please suggest further processing on this, if needed.

Thanks.

Hi Nitin,

Thanks for the patch! I left a comment on the review with some additional information regarding the hints object. I also included a diff of another test case we could possibly add to that module (http://cdn.pasteraw.com/pp626dyueny2aiprvo15uankbus97nf).

You should be able to apply that diff using:

curl http://cdn.pasteraw.com/pp626dyueny2aiprvo15uankbus97nf | git apply

Let me know what you think of the addition to the patch.

I don't think we should move this to Fix Released just yet. The original bug report is asking for the ability to build a hints object that asks for specific values to be None. The tests in the patch [0] are only asserting that we get all credentials back when we don't pass hints, and that we get all credentials back when we pass a hints value of None (which is working as designed - and I probably should have raised this in review sooner).

What I could have probably been a little more clear on was that we should have the ability to build a hints object asking for specific attributes of an entity to be None. Something like the following:

  from keystone.common import driver_hints
  ...
  hints = driver_hints.Hints()
  hints.add_filter('project_id', None)
  credentials = self.credential_api.list_credentials(hints=hints)

The above example is explicitly asking for a subset of an entity collection where the only returned entities have a project_id attribute set to None, or not set at all.

The behavior I was seeing when working on the encrypted credentials blueprint in Newton, was that I was unable to get hints to do exactly that. I was able to build a hints object, just like I did above, but I wasn't able to get it to return the entities with None as the value of the attribute I was looking for (at the time i was looking for all credential references with `key_hash` set to None, see my comment inline for more details on why I wanted to do this [1]).

Does this help clarify?

[0] https://github.com/openstack/keystone/blob/52c2a810bd62ffe1953441af9784d748d28645cc/keystone/tests/unit/credential/test_backend_sql.py#L85-L91
[1] https://github.com/openstack/keystone/blob/e28dddda024f898eece83c67374cafd742a9e309/keystone/cmd/cli.py#L724-L731