Skip to content

Conversation

@f-necas
Copy link
Collaborator

@f-necas f-necas commented Aug 28, 2025

By default attributes are retrieved from source layer (even if checkbox is not set) and copied to dest layer. But It check the box by default in dest layer.

This PR aims to remove it. Solves #94

There's 2 cases:

  • If layer doesn't exist and attributes aren't stored anywhere (e.g in database), we need to send the attribute list in order to not hit a 400 from geoserver rying to create new feature type inside the store, but no attributes were specified org.geoserver.rest.RestException 400 BAD_REQUEST: Trying to create new feature type inside the store, but no attributes were specified
  • If layer doesn't exist but attributes exist (we often see a "Publish" when we select the layer next to it when we want to add a new layer), we need to remove the attributes part from the xml.
image

@f-necas f-necas force-pushed the remove-attributes branch from 20d11ff to c53e36f Compare August 28, 2025 07:32
Copy link
Contributor

@mki-c2c mki-c2c left a comment

Choose a reason for hiding this comment

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

Thanks.
Great analysis of the API behaviour, clean code.

I would prefer using lxml as library for consistency. However, I am sorry about asking for this change, it's doing the work again and not sooo important.

Please decide by yourself if you would like to change the library or not.

@f-necas f-necas force-pushed the remove-attributes branch from 1992b87 to 8ba2495 Compare August 28, 2025 09:31
@f-necas f-necas requested a review from mki-c2c August 28, 2025 09:40
Copy link
Contributor

@mki-c2c mki-c2c left a comment

Choose a reason for hiding this comment

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

Perfect, I think you were able to test the corner cases, I just found some old error handling code which might never be reached, but it does not hurt

raise ParamError(
context="dst",
key=resource_post_route,
err="Route not found. Check Workspace and datastore",
Copy link
Contributor

Choose a reason for hiding this comment

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

Normally workspace should have been checked beforehand:
https://github.com/georchestra/maelstro/blob/main/backend/maelstro/core/copy_manager.py#L234

So if we get a 404 here, it should not be a problem of workspace. Probably this has become dead code thanks to the workspace check

@f-necas f-necas merged commit 44ed608 into main Aug 28, 2025
3 checks passed
@f-necas f-necas deleted the remove-attributes branch August 28, 2025 10:36
github-actions bot pushed a commit that referenced this pull request Aug 28, 2025
…m attributes checkbox set (#97)

* feat: remove attributes elements before featureType insertion to avoid custom attributes checkbox set

* feat: add try except to catch featureAttributes not existing + use lxml

(cherry picked from commit 44ed608)
@github-actions
Copy link

💚 All backports created successfully

Status Branch Result
0.1.x

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

f-necas added a commit that referenced this pull request Aug 28, 2025
…m attributes checkbox set (#97) (#98)

* feat: remove attributes elements before featureType insertion to avoid custom attributes checkbox set

* feat: add try except to catch featureAttributes not existing + use lxml

(cherry picked from commit 44ed608)

Co-authored-by: f-necas <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants