[Fixes #10537] Improve rules creation using GeoFence batch #10538

etj · 2023-01-16T10:13:57Z

The creation of the Rules in GeoFence should be as quick as possibile.
In order to improve the performance of rule creation, we can leverage on the use of GeoFence batch.

See whole rationale on #10537.

Checklist

Reviewing is a process done by project maintainers, mostly on a volunteer basis. We try to keep the overhead as small as possible and appreciate if you help us to do so by completing the following items. Feel free to ask in a comment if you have troubles with any of them.

For all pull requests:

Confirm you have read the contribution guidelines
You have sent a Contribution Licence Agreement (CLA) as necessary (not required for small changes, e.g., fixing typos in the documentation)
Make sure the first PR targets the master branch, eventual backports will be managed later. This can be ignored if the PR is fixing an issue that only happens in a specific branch, but not in newer ones.

The following are required only for core and extension modules (they are welcomed, but not required, for contrib modules):

There is a ticket in https://github.com/GeoNode/geonode/issues describing the issue/improvement/feature (a notable exemption is, changes not visible to end-users)
The issue connected to the PR must have Labels and Milestone assigned
PR for bug fixes and small new features are presented as a single commit
Commit message must be in the form "[Fixes #<issue_number>] Title of the Issue"
New unit tests have been added covering the changes, unless there is an explanation on why the tests are not necessary/implemented
This PR passes all existing unit tests (test results will be reported by travis-ci after opening this PR)
This PR passes the QA checks: flake8 geonode
Commits changing the settings, UI, existing user workflows, or adding new functionality, need to include documentation updates
Commits adding new texts do use gettext and have updated .po / .mo files (without location infos)

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.

mattiagiupponi · 2023-01-16T11:28:00Z

@etj can you please fix the flake8 issues so we can check if the tests works?

geonode/geoserver/security.py

geonode/geoserver/geofence.py

afabiani · 2023-01-17T09:14:58Z

There's a test failing

======================================================================
ERROR: test_sync_resources_with_guardian_delay_true (geonode.security.tests.SecurityRulesTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/src/geonode/geonode/geoserver/geofence.py", line 223, in insert_rule
    raise GeofenceException(f"Could not insert rule: [{r.status_code}]")
geonode.geoserver.geofence.GeofenceException: Could not insert rule: [500]

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/src/geonode/geonode/geoserver/security.py", line 299, in set_geofence_all
    client.insert_rule(Rule(priority, workspace, dataset_name, Rule.ALLOW))
  File "/usr/src/geonode/geonode/geoserver/geofence.py", line 227, in insert_rule
    raise GeofenceException(f"Error while inserting rule: {e}")
geonode.geoserver.geofence.GeofenceException: Error while inserting rule: Could not insert rule: [500]

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/src/geonode/geonode/security/tests.py", line 2175, in test_sync_resources_with_guardian_delay_true
    set_geofence_all(self._l)
  File "/usr/src/geonode/geonode/geoserver/security.py", line 303, in set_geofence_all
    raise RuntimeError(f"Could not ADD GeoServer ANONYMOUS Rule for Dataset {dataset_name}: {e}")
RuntimeError: Could not ADD GeoServer ANONYMOUS Rule for Dataset test_dataset: Error while inserting rule: Could not insert rule: [500]

afabiani

Other than the test failure and the comments from @mattiagiupponi the functional tests are ok.

geonode/geoserver/geofence.py

geonode/geoserver/security.py

…7-geofence-batch

codecov · 2023-01-18T17:23:33Z

Codecov Report

Merging #10538 (e49ebcb) into master (33d4aa9) will decrease coverage by 0.03%.
The diff coverage is 78.57%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #10538      +/-   ##
==========================================
- Coverage   62.14%   62.11%   -0.03%     
==========================================
  Files         827      828       +1     
  Lines       51133    51141       +8     
  Branches     6562     6557       -5     
==========================================
- Hits        31775    31767       -8     
- Misses      17673    17691      +18     
+ Partials     1685     1683       -2

* [Fixes #10537] Improve rules creation using GeoFence batch * - code improvements accordingly to the PR comments * - code improvements accordingly to the PR comments * - Test fixes Co-authored-by: afabiani <[email protected]> (cherry picked from commit d101ead)

…10567) * [Fixes #10537] Improve rules creation using GeoFence batch * - code improvements accordingly to the PR comments * - code improvements accordingly to the PR comments * - Test fixes Co-authored-by: afabiani <[email protected]> (cherry picked from commit d101ead) Co-authored-by: Emanuele Tajariol <[email protected]>

etj · 2023-01-23T15:38:14Z

geonode/br/management/commands/restore.py

+                                static_files_folder.startswith(settings.PROJECT_ROOT):
                            print(f"[Sanity Check] Full Write Access to '{static_files_folder}' ...")
                            chmod_tree(static_files_folder)
                    for template_files_folder in template_folders:
                        if getattr(settings, 'PROJECT_ROOT', None) and \
-                                    template_files_folder.startswith(settings.PROJECT_ROOT):
+                                template_files_folder.startswith(settings.PROJECT_ROOT):
                            print(f"[Sanity Check] Full Write Access to '{template_files_folder}' ...")
                            chmod_tree(template_files_folder)
                    for locale_files_folder in locale_folders:
                        if getattr(settings, 'PROJECT_ROOT', None) and \
-                                    locale_files_folder.startswith(settings.PROJECT_ROOT):
+                                locale_files_folder.startswith(settings.PROJECT_ROOT):
                            print(f"[Sanity Check] Full Write Access to '{locale_files_folder}' ...")
                            chmod_tree(locale_files_folder)
                except Exception as exception:


these changes should not be in this PR

etj · 2023-01-23T15:39:56Z

geonode/geoserver/geofence.py

@@ -128,11 +229,11 @@ def __init__(self, baseurl: str, username: str, pw: str) -> None:

    def invalidate_cache(self):
        r = requests.put(
-            f'{self.baseurl}rest/geofence/ruleCache/invalidate',
+            f'{self.baseurl.rstrip("/")}/rest/geofence/ruleCache/invalidate',


this change (and next similar ones) was better implemented in the next geofence PR.
It will be replaced

…oNode#10538) (GeoNode#10567) * [Fixes GeoNode#10537] Improve rules creation using GeoFence batch * - code improvements accordingly to the PR comments * - code improvements accordingly to the PR comments * - Test fixes Co-authored-by: afabiani <[email protected]> (cherry picked from commit d101ead) Co-authored-by: Emanuele Tajariol <[email protected]>

etj assigned giohappy and afabiani Jan 16, 2023

etj linked an issue Jan 16, 2023 that may be closed by this pull request

Improve rules creation using GeoFence batch #10537

Closed

etj assigned mattiagiupponi Jan 16, 2023

cla-bot bot added the cla-signed CLA Bot: community license agreement signed label Jan 16, 2023

etj force-pushed the 10537-geofence-batch branch 2 times, most recently from 68952c2 to 80afd5d Compare January 16, 2023 12:04

[Fixes #10537] Improve rules creation using GeoFence batch

cf8fc09

etj force-pushed the 10537-geofence-batch branch from 80afd5d to cf8fc09 Compare January 16, 2023 12:42

mattiagiupponi requested changes Jan 16, 2023

View reviewed changes

afabiani requested changes Jan 17, 2023

View reviewed changes

geonode/geoserver/geofence.py Outdated Show resolved Hide resolved

geonode/geoserver/security.py Outdated Show resolved Hide resolved

etj mentioned this pull request Jan 17, 2023

[Fixes #10537] Improve rules creation using GeoFence batch - further improvements #10556

Closed

12 tasks

afabiani added 3 commits January 18, 2023 12:59

Merge branch 'master' of https://github.com/GeoNode/geonode into 1053…

d8ffd5b

…7-geofence-batch

- code improvements accordingly to the PR comments

9ef541f

- code improvements accordingly to the PR comments

3a16797

afabiani previously approved these changes Jan 18, 2023

View reviewed changes

- Test fixes

e49ebcb

afabiani dismissed their stale review via e49ebcb January 18, 2023 16:46

afabiani requested a review from mattiagiupponi January 18, 2023 17:42

afabiani added the backport 4.1.x label Jan 18, 2023

afabiani added this to the 4.1.0 milestone Jan 18, 2023

afabiani approved these changes Jan 18, 2023

View reviewed changes

afabiani merged commit d101ead into master Jan 23, 2023

afabiani deleted the 10537-geofence-batch branch January 23, 2023 10:20

github-actions bot mentioned this pull request Jan 23, 2023

[Backport 4.1.x] [Fixes #10537] Improve rules creation using GeoFence batch #10567

Merged

etj commented Jan 23, 2023

View reviewed changes

etj mentioned this pull request Jan 25, 2023

[Fixes #10537] Improve rules creation using GeoFence batch - more imp… #10585

Merged

12 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Fixes #10537] Improve rules creation using GeoFence batch #10538

[Fixes #10537] Improve rules creation using GeoFence batch #10538

etj commented Jan 16, 2023 •

edited by afabiani

Loading

mattiagiupponi commented Jan 16, 2023 •

edited

Loading

afabiani commented Jan 17, 2023

afabiani left a comment

codecov bot commented Jan 18, 2023 •

edited

Loading

etj Jan 23, 2023

etj Jan 23, 2023

[Fixes #10537] Improve rules creation using GeoFence batch #10538

[Fixes #10537] Improve rules creation using GeoFence batch #10538

Conversation

etj commented Jan 16, 2023 • edited by afabiani Loading

Checklist

mattiagiupponi commented Jan 16, 2023 • edited Loading

afabiani commented Jan 17, 2023

afabiani left a comment

Choose a reason for hiding this comment

codecov bot commented Jan 18, 2023 • edited Loading

Codecov Report

etj Jan 23, 2023

Choose a reason for hiding this comment

etj Jan 23, 2023

Choose a reason for hiding this comment

etj commented Jan 16, 2023 •

edited by afabiani

Loading

mattiagiupponi commented Jan 16, 2023 •

edited

Loading

codecov bot commented Jan 18, 2023 •

edited

Loading