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

Layergroup creation bugfix #97

Merged
merged 3 commits into from
Feb 8, 2023
Merged

Layergroup creation bugfix #97

merged 3 commits into from
Feb 8, 2023

Conversation

julescarpentier
Copy link

Hi,

Geoserver now returns 404 HTTP error code when requesting for a resource that doesn't exists (instead of 200 OK with empty JSON before I imagine)

This impacts the layergroup creation method as it checks a layergroup with the same name doesn't exists already.

Best regards

@iamtekson
Copy link
Collaborator

That's the correct status code for non-existing layergroups. I think it is not necessary to bypass the 404 error.

@julescarpentier
Copy link
Author

julescarpentier commented Jan 31, 2023

I agree, 404 is the correct status code for non-existing layergroups.
If no bypass, then the check for an existing layergroup with that name should be removed. Because as it is, when creating a new layergroup, the check expects None from get_layergroup() yet this function raises an Exception since the layer doesn't exist.

if self.get_layergroup(name) is not None:
raise Exception(
f"Layergroup: {name} already exist in Geoserver instance"
)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure whether it is smart to remove the check entirely, because this will cause issues for any code that assumes it will get an error if the layergroup exists already.

I would suggest to check for an exception in get_layergroup, like (I don't know if this is the most pythonic solution, but it works):

            # check if it already exist in Geoserver
            try: 
                existing_layer_group = self.get_layergroup(name)
            except:
                existing_layer_group = None

            if existing_layer_group is not None:
                raise Exception(
                    f"Layergroup: {name} already exists in Geoserver instance"
                )

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, I think it is the correct way of fixing for now. Can you please update this PR with above suggestion? @julescarpentier

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

Successfully merging this pull request may close these issues.

None yet

3 participants