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.

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

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

Merged

Conversation

seitenbau-govdata
Copy link
Member

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 seitenbau-govdata marked this pull request as ready for review May 26, 2023 13:46
@seitenbau-govdata
Copy link
Member Author

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

@amercader
Copy link
Member

@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 seitenbau-govdata changed the title Use tuples for dictization of harvest job errors Adapt template to latest changes in harvest job error dictization Jun 1, 2023
@seitenbau-govdata
Copy link
Member Author

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

@amercader amercader merged commit 02aeaf4 into ckan:master Jun 1, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants