Bug 1011053
| Summary: | Criteria.as_dict() says its output can be used as input to Criteria.from_client_input(), but it fails validation | ||
|---|---|---|---|
| Product: | [Retired] Pulp | Reporter: | Randy Barlow <rbarlow> |
| Component: | z_other | Assignee: | Randy Barlow <rbarlow> |
| Status: | CLOSED CURRENTRELEASE | QA Contact: | Preethi Thomas <pthomas> |
| Severity: | medium | Docs Contact: | |
| Priority: | unspecified | ||
| Version: | Master | CC: | pthomas |
| Target Milestone: | --- | ||
| Target Release: | 2.4.0 | ||
| Hardware: | All | ||
| OS: | All | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | Bug Fix | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2014-08-09 06:55:50 UTC | Type: | Bug |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
We could make a from_dict() method that is able to to consume the output of the to_dict() method as a solution to this bug. Then the from_client_input() method could make use of from_dict() after doing its validation and type conversion. This will be important for our distributed task conversion work. We could also consider not using the Criteria model anymore, and just use dictionaries. As far as I can tell, this is pretty much just a data structure that can convert itself from client input and to dictionaries. It does also have a "spec" property, but it doesn't have a docblock so I'm not very clear about what it does. If we can get rid of this class, I think it will make some of our code more maintainable and easier to understand to to greater simplicity. I am fixing this by making a from_dict(). This was fixed in a prior release of Pulp, but we never put it through the QE process. Moving to ON_QA. Preethi, This bug is difficult to verify using Pulp's features, as it is an internal refactor that was necessary for the new tasks to work in 2.4.0. There is a way to verify that it works in a Python shell: rbarlow@guava ~/d/p/pulp> python Python 2.7.6 (default, May 27 2014, 16:39:58) [GCC 4.9.0 20140518 (Red Hat 4.9.0-5)] on linux2 Type "help", "copyright", "credits" or "license" for more information. >>> from pulp.server.db.model.criteria import Criteria >>> c1 = Criteria(filters={'name': 'test'}) >>> c1 {'sort': None, 'skip': None, 'limit': None, 'filters': {'name': 'test'}, 'fields': None, '_id': ObjectId('53ac21a5e5147d1119496316'), 'id': '53ac21a5e5147d1119496316'} >>> c2 = Criteria.from_dict(c1.as_dict()) >>> c2 {'sort': None, 'skip': None, 'limit': None, 'filters': {'name': 'test'}, 'fields': None, '_id': ObjectId('53ac21bfe5147d1119496317'), 'id': '53ac21bfe5147d1119496317'} If you can create c2 from c1, using c1's as_dict() method and Criteria's from_dict() static method as shown above, this bug is solved. verified [root@yttrium ~]# rpm -qa pulp-server pulp-server-2.4.0-0.23.beta.el6.noarch [root@yttrium ~]# [root@yttrium ~]# python Python 2.6.6 (r266:84292, Sep 4 2013, 07:46:00) [GCC 4.4.7 20120313 (Red Hat 4.4.7-3)] on linux2 Type "help", "copyright", "credits" or "license" for more information. >>> from pulp.server.db.model.criteria import Criteria >>> c1 = Criteria(filters={'name': 'test'}) >>> c1 {'sort': None, 'skip': None, 'limit': None, 'filters': {'name': 'test'}, 'fields': None, '_id': ObjectId('53ad650a7bcdb34a1e2a20eb'), 'id': '53ad650a7bcdb34a1e2a20eb'} >>> c2 = Criteria.from_dict(c1.as_dict()) >>> c2 {'sort': None, 'skip': None, 'limit': None, 'filters': {'name': 'test'}, 'fields': None, '_id': ObjectId('53ad65277bcdb34a1e2a20ec'), 'id': '53ad65277bcdb34a1e2a20ec'} >>> This has been fixed in Pulp 2.4.0-1. |
The Criteria.as_dict() docblock says that its output is "compatible as input to the from_client_input method." However, if you create a Criteria object, serialize it with as_dict(), and then try to create a new one with from_client_input(), it will fail validation. For example, I've converted the consumer applicability regeneration code to use this method to send the input through a message broker, and on the other end I get this traceback: Traceback (most recent call last): File "/home/rbarlow/devel/pulp/server/test/unit/test_server_managers_consumer_applicability.py", line 170, in test_regenerate_applicability_for_consumer_criteria_no_bindings manager.regenerate_applicability_for_consumers(self.CONSUMER_CRITERIA) File "/home/rbarlow/devel/pulp/server/pulp/server/managers/consumer/applicability.py", line 44, in regenerate_applicability_for_consumers consumer_criteria = Criteria.from_client_input(consumer_criteria) File "/home/rbarlow/devel/pulp/server/pulp/server/db/model/criteria.py", line 80, in from_client_input sort = _validate_sort(doc.pop('sort', None)) File "/home/rbarlow/devel/pulp/server/pulp/server/db/model/criteria.py", line 318, in _validate_sort if not isinstance(entry[0], basestring): KeyError: 0 This happens because the sort attribute looks like this: [{'id': 1}] It's already been converted from user input to a dictionary, but the validation code is assuming that it's user input (i.e., using words like "ascending").