ckan / ckanext-harvest

Remote harvesting extension for CKAN
130 stars 203 forks source link

Adapt template to latest changes in harvest job error dictization #533

Closed seitenbau-govdata closed 1 year ago

seitenbau-govdata commented 1 year ago

In #529 we changed the dictization of harvest errors. While this solves the problem #528 it causes a problem with displaying the error messages in the job overview. Therefore we would fix this by using a tuple to restore the old behavior.

Details Since the update to sqlalchemy 1.4.x the harvest error objects returned by the database in https://github.com/ckan/ckanext-harvest/blob/master/ckanext/harvest/logic/dictization.py#L79 are no longer serializable which causes commands like harvester reindex or harvester clearsource_history to fail if there are any harvest jobs with errors. In #529 these harvest error objects are transformed into a dict to fix this problem. By doing so this data can no longer be accessed by index but this is expected by the HTML template (https://github.com/ckan/ckanext-harvest/blob/master/ckanext/harvest/templates/snippets/job_error_summary.html) and potentially by further CKAN extensions. So our proposed solution would be to use a tuple for the data to regain the old behavior and still fix the serialization problems.

seitenbau-govdata commented 1 year ago

Could somebody take a quick look if this is a viable solution? Maybe @amercader or @pdelboca

amercader commented 1 year ago

@seitenbau-govdata I think it's very unlikely that other extensions are using this object so I'd rather update the templates to use a dict than return a tuple. Dicts are the preferred output format for the API anyway

seitenbau-govdata commented 1 year ago

@amercader Thanks for the feedback. We've made a new commit with changes to the template.