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

Get a package last modifier based on the package name instead of the package ID to avoid a ckan.logic.NotFound exception #82

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ntwalibas
Copy link
Contributor

Description

Running pipelines in who-afro-dags resulted in certain packages not being found by their IDs even though they clearly exist.
This PR changes the way we get a package last activity where we rely on the package name instead of the package ID which fixes the afore mentioned issue.

Testing

This PR was tested manually instead of making an automated test as I still do not know how to reliably reproduce the issue
that resulted in this work.

Checklist

  • The Jira ticket for this issue has been updated to "Ready to Review" or equivalent.
  • I have developed these changes in discussion with the appropriate project manager.
  • My code follows the general Fjelltopp documentation (see Confluence).
  • I have made corresponding changes to the Fjelltopp documentation (see Confluence).
  • I have rebased this branch with master.
  • New dependency changes have been committed.
  • I have added automated tests that prove my fix is effective or that my feature works.
  • New and existing tests pass locally with my changes.
  • My changes generate no new warnings.
  • I have performed a self-review of my own code.
  • I have assigned at least one reviewer.
  • I have assigned at least one label to this PR: "patch", "minor", "major".

…package ID to avoid a ckan.logic.NotFound exception.
Copy link
Member

@ChasNelson1990 ChasNelson1990 left a comment

Choose a reason for hiding this comment

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

Why does get_last_modifier break with id but apparently get_package_stats is fine?

@ntwalibas
Copy link
Contributor Author

ntwalibas commented Oct 10, 2024

@ChasNelson1990 said:

Why does get_last_modifier break with id but apparently get_package_stats is fine?

On one hand:
The Google analytics helper get_package_stats returns 0 if no package with the given ID could be found, which is the case for the problematic packages. Then once a package is added to the package table, a corresponding entry is created in the activity table. Therefore if no entry can be found in the activity table given the package ID (object_id column in the DB), an exception is raised when the package_activity_list action is invoked (which is the function called by get_last_modifier).

Also, the action package_activity_list allows you to find packages either by their name or by their ID, get_package_stats only relies on the ID.

On the other hand:
get_package_stats will display the wrong results once Google analytics has captured data about the package because there will be an inconsistency between the package_id stored in the package_stats table and the actual package ID to be found in the package table.

Good observation! It seems I got so much tunnel vision, I didn't consider the ramifications of using the package id in certain places and the package name in other places.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Datasets in logged in user dashboard resulting in ckan.logic.NotFound exception
2 participants