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

Feat/handle error deleting old resource for main #139

Merged
merged 11 commits into from
Dec 4, 2024

Conversation

bellisk
Copy link
Contributor

@bellisk bellisk commented Dec 3, 2024

No description provided.

If we get into a state where we have duplicate resources for the same filename (e.g. because we created a new resource for the filename but deleting the old resource failed), we must not delete resources by filename in the finalize step! If we do, we will delete both the new and the old resource for the filename.

This method is already a lot simpler than it used to be, since CKAN no longer stores lots of old versions of resources in the database, so it's safe to change it to delete resources by id.
This will also clean up the filestore of old versions of files that are
otherwise not cleaned up.
@bellisk bellisk force-pushed the feat/handle-error-deleting-old-resource-for-main branch from 29cf1a9 to 86f281f Compare December 3, 2024 16:19
v1.6.0 included changes that required updates on our side, which were made in the migration branch of ckanext-switzerland. No point adding the same changes here as the migration will soon be finished and the migration branch will be merged into main.
I forgot that we are using our own fork here instead of the official ckan one.
@bellisk bellisk marked this pull request as ready for review December 3, 2024 16:51
@bellisk bellisk merged commit 0e21d01 into main Dec 4, 2024
4 checks passed
@bellisk bellisk deleted the feat/handle-error-deleting-old-resource-for-main branch December 4, 2024 09:13
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.

2 participants