Cumulative backport to 5.1.x of GWC PRs#14379
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the GWC tile layer creation command by deprecating the old create_tile_layers command and integrating it as a subcommand under the unified gwc command. It also updates GWCClient with GET and PUT methods, introduces an XML template for layer creation, and adds support for dry-run and debug modes. Feedback has been provided to improve error handling and resilience in the new create subcommand, specifically by moving the layer retrieval call inside the try...except block to prevent crashes on network failures, handling undefined variables during dry runs, and filtering out empty layer names to avoid potential TypeErrors.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| r = gwc.get_layer(layername) | ||
| if r.status_code == 200: | ||
| if force: | ||
| logger.info(" - Forcing layer configuration in GWC") | ||
| cnt_force += 1 | ||
| else: | ||
| logger.info(f" - Layer already configured in GWC (code {r.status_code})") | ||
| cnt_old += 1 | ||
| continue | ||
| try: | ||
| logger.info(" - Configuring...") | ||
| data = self.generate_xml(template, layername) | ||
| if not dry_run: | ||
| response = gwc.set_layer(layername, data=data) | ||
|
|
||
| if dry_run or response.status_code == 200: | ||
| logger.info(f" - Done {layername}") | ||
| cnt_new += 1 | ||
| else: | ||
| logger.warning(f"Layer {layername} couldn't be configured: code {response.status_code}") | ||
| cnt_bad += 1 | ||
|
|
||
| except Exception as e: | ||
| logger.warning(f"Error processing {layername}: {e}") | ||
| cnt_bad += 1 |
There was a problem hiding this comment.
The gwc.get_layer(layername) call is currently placed outside the try...except block. If any network timeout, connection error, or API failure occurs during this call, the entire management command will crash and terminate immediately, rather than logging the error and continuing to process the remaining layers. Moving this call inside the try...except block makes the loop much more resilient. Additionally, initializing response = None and checking response before accessing response.status_code prevents potential NameError or AttributeError when dry_run is True.
try:
r = gwc.get_layer(layername)
if r.status_code == 200:
if force:
logger.info(" - Forcing layer configuration in GWC")
cnt_force += 1
else:
logger.info(f" - Layer already configured in GWC (code {r.status_code})")
cnt_old += 1
continue
logger.info(" - Configuring...")
data = self.generate_xml(template, layername)
response = None
if not dry_run:
response = gwc.set_layer(layername, data=data)
if dry_run or (response and response.status_code == 200):
logger.info(f" - Done {layername}")
cnt_new += 1
else:
status_code = response.status_code if response else "N/A"
logger.warning(f"Layer {layername} couldn't be configured: code {status_code}")
cnt_bad += 1
except Exception as e:
logger.warning(f"Error processing {layername}: {e}")
cnt_bad += 1| if create_all: | ||
| datasets_fields = Dataset.objects.values_list('typename', 'alternate') | ||
| layer_names = [typename or alternate for typename, alternate in datasets_fields] | ||
| else: | ||
| layer_names = requested_layers |
There was a problem hiding this comment.
If create_all is True, Dataset.objects.values_list('typename', 'alternate') is queried. If any dataset has both fields as None or empty, it will add None to layer_names, which subsequently causes a TypeError in _validate_layer_name. Filtering out falsy values prevents this. Additionally, if requested_layers is None (the default) and create_all is False, layer_names will be None, causing len(layer_names) to raise a TypeError. Defaulting to an empty list requested_layers or [] ensures robustness.
| if create_all: | |
| datasets_fields = Dataset.objects.values_list('typename', 'alternate') | |
| layer_names = [typename or alternate for typename, alternate in datasets_fields] | |
| else: | |
| layer_names = requested_layers | |
| if create_all: | |
| datasets_fields = Dataset.objects.values_list('typename', 'alternate') | |
| layer_names = [typename or alternate for typename, alternate in datasets_fields if typename or alternate] | |
| else: | |
| layer_names = requested_layers or [] |
There was a problem hiding this comment.
Pull request overview
Backports GWC-related improvements to the 5.1.x branch by expanding the gwc management command into a subcommand-based interface and aligning tile-layer creation with an XML template, while deprecating the legacy create_tile_layers command.
Changes:
- Add
gwc createsubcommand and refactorgwc truncateinto a subparser-based CLI with shared--dry-run/--debugflags. - Introduce a templated XML-driven tile-layer creation workflow (
gwc_subcommands/create.py+create_template.xml). - Extend
GWCClientwith REST helpers (GET/PUT) and layer get/set operations; route deprecatedcreate_tile_layersto the new implementation.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| geonode/geoserver/management/commands/gwc.py | Converts gwc into a subcommand-based CLI and wires create + truncate. |
| geonode/geoserver/management/commands/gwc_subcommands/truncate.py | Moves truncate argument parsing into the subcommand module and adds dry-run/debug plumbing. |
| geonode/geoserver/management/commands/gwc_subcommands/create.py | Adds tile-layer creation subcommand using an XML template and new GWCClient methods. |
| geonode/geoserver/management/commands/gwc_subcommands/create_template.xml | Adds/aligns the GWC layer XML template (gridsets + metatiling). |
| geonode/geoserver/gwc.py | Adds debug-aware REST helpers plus get_layer/set_layer. |
| geonode/br/management/commands/create_tile_layers.py | Deprecates the legacy command and dispatches to gwc create implementation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if create_all: | ||
| datasets_fields = Dataset.objects.values_list('typename', 'alternate') | ||
| layer_names = [typename or alternate for typename, alternate in datasets_fields] | ||
| else: | ||
| layer_names = requested_layers |
| force = options.get('force') | ||
| requested_layers = options.get('layers') | ||
| dry_run = options.get('dry-run') | ||
| logger.error("THIS COMMAND IS DEPRECATED - USE THE gwc COMMAND INSTEAD") |
| def handle(self, **options): | ||
| dry_run = options.get('dry-run') | ||
| debug = options.get('debug') | ||
| setup_logger(level=logging.DEBUG if debug else logging.INFO) | ||
|
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## 5.1.x #14379 +/- ##
========================================
Coverage ? 74.80%
========================================
Files ? 984
Lines ? 60525
Branches ? 8268
========================================
Hits ? 45275
Misses ? 13399
Partials ? 1851 🚀 New features to boost your workflow:
|
This PR contains the backport of some PRs that failed to be automatically backported:
[Fixes #14306] Refact create_tile_layers as a subcommand of gwc #14310: [Fixes Refact create_tile_layers as a subcommand of gwc #14306] Refact create_tile_layers as a subcommand of gwc
This PR is composed by 3 commits, but automatic backport only backported the last one ([Backport 5.1.x] [Fixes #14306] Refact create_tile_layers as a subcommand of gwc #14313)
[Fixes #14358] Align GWC template to the default GWC configuration #14378: [Fixes Align GWC template to the default GWC configuration #14358] Align GWC template to the default GWC configuration
The backport of this PR failed bc the underlying files to be patched did not exist, since they were initially created in the 14310 PR.
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.