[Fixes #13828] Return dirty resources inside endpoints #13830
Conversation
Summary of ChangesHello @sijandh35, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses an issue where 'dirty' resources were not consistently returned or tracked within GeoNode's API and resource management system. It modifies the default behavior of several core functions to ensure that dirty resources are included by default in API responses and visibility checks, and improves their tracking during asynchronous processing. This change aims to provide a more complete view of resources, including those in a 'dirty' or pending state, to API consumers. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly addresses the issue of not returning 'dirty' resources from API endpoints by changing the default behavior to include them. The modifications in geonode/layers/api/permissions.py and geonode/security/utils.py are consistent with this goal. The addition to geonode/upload/celery_tasks.py is also a good improvement, ensuring that the status of copy operations can be tracked via the original resource's UUID. I've provided one suggestion in geonode/layers/api/permissions.py to improve the exception handling by catching a more specific error, which will make the code more robust.
| try: | ||
| include_dirty = strtobool(request.query_params.get("include_dirty", "False")) | ||
| include_dirty = strtobool(request.query_params.get("include_dirty", "True")) | ||
| except Exception: | ||
| include_dirty = False | ||
| include_dirty = True |
There was a problem hiding this comment.
It's good practice to catch more specific exceptions rather than the general Exception. The strtobool function raises a ValueError for invalid inputs. Catching ValueError specifically makes the code more robust and avoids masking other potential bugs.
| try: | |
| include_dirty = strtobool(request.query_params.get("include_dirty", "False")) | |
| include_dirty = strtobool(request.query_params.get("include_dirty", "True")) | |
| except Exception: | |
| include_dirty = False | |
| include_dirty = True | |
| try: | |
| include_dirty = strtobool(request.query_params.get("include_dirty", "True")) | |
| except ValueError: | |
| include_dirty = True |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@ ## master #13830 +/- ## =========================================== + Coverage 52.15% 74.13% +21.98% =========================================== Files 944 944 Lines 56335 56336 +1 Branches 7626 7626 =========================================== + Hits 29380 41765 +12385 + Misses 25588 12904 -12684 - Partials 1367 1667 +300 🚀 New features to boost your workflow:
|
Fixes #13828
This PR returns dirty state resources inside endpoints and also added execution info for original dirty state resource.
Note:
During cloning process, if we quickly refresh it after cloning it shows the following error and refreshing again after some period it will go, this behaviour seen when checked from admin.
It might be due to resource_type null for newly created resource on dirty state, so might need be handled from the client side for this error.
Checklist
For all pull requests:
The following are required only for core and extension modules (they are welcomed, but not required, for contrib modules):
Submitting the PR does not require you to check all items, but by the time it gets merged, they should be either satisfied or inapplicable.