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.

Sign up for GitHub

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

Jump to bottom

Do not crash on AttributeError in dict_list_reduce helper function #2684

Closed
wants to merge 1 commit into from
Closed

Do not crash on AttributeError in dict_list_reduce helper function #2684

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion ckan/lib/helpers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -868,7 +868,11 @@ def dict_list_reduce(list_, key, unique=True):
values for the key with unique values if requested. '''
new_list = []
for item in list_:
value = item.get(key)
try:
value = item.get(key)
except AttributeError:
# item might not be a dict.
continue
Copy link
Contributor

Choose a reason for hiding this comment

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

As the function is documented as taking a list of dicts, it's possibly not the right thing to do, to just silently ignore the bad data passed in. Perhaps if you raise an error that points out that it has been passed bad data (e.g. not a list of dicts) may help in tracking down the real cause of the problem (possibly in a schema somewhere?).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ross Jones wrote:

As the function is documented as taking a list of dicts, it's possibly
not the right thing to do, to just silently ignore the bad data passed
in. Perhaps if you raise an error that points out that it has been
passed bad data (e.g. not a list of dicts) may help in tracking down the
real cause of the problem (possibly in a schema somewhere?).

I agree this is not the right thing to do (just did not read the
docstring :P). I can debug this live if you point me to something to
look for. The full traceback does not look very helpful from my point of
view...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It appears that the package.resources object passed to dict_list_reduce is actually not a list but a dict (corresponding to one resource). But the package at stake actually has two resources.

Copy link
Contributor

Choose a reason for hiding this comment

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

~~I think the thing to investigate is why when calling dict_list_reduce with

h.dict_list_reduce(package.resources, 'format')

we get things that are not resource dicts. package.resources should definitely be a list of dicts.

Perhaps just dumping {{package.resources}} in the package_item template might show what it is being passed, I can only imagine that a plugin/custom-schema is interfering somehow.~~

There's definitely some schema weirdness going on. Do you have a custom plugin? Which plugins do you use?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I'm using ckanext-dcat and this dataset comes from a DCAT harvest.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

package.resources is just one dict which looks like:

{0: {u'cache_last_updated': None, 
       u'issued': u'2015-10-13T15:00:17.332357', 
       u'uri': u'http://localhost:8080/distribution/1301',
       u'package_id': u'6b0af45b-81c5-425c-bcb2-42f291937ae7',
      ...}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

This is weird, perhaps @amercader might know if this is a dcat extension specific weirdness.

As a temporary workaround, you can probably get away with (obviously untested)..

h.dict_list_reduce(package.resources.values(), 'format')

but it should only be a temporary workaround.

if not value or (unique and value in new_list):
continue
new_list.append(value)
Expand Down