gicait / geoserver-rest

Python library for management of geospatial data in GeoServer.
https://geoserver-rest.readthedocs.io
MIT License
197 stars 79 forks source link

Can we raise GeoserverException instead of python native Exception? #104

Closed Wrufesh closed 1 year ago

Wrufesh commented 1 year ago

Hi,

I just started using geoserver-rest for one of my script which does some preprocessing of dataset and uploads them to geoserver.

I was interested in organizing my uploads with proper namespace. So in case the workspace "ADCD" already exist and I do:

geo.create_workspace(workspace='ABCD')

I get:

Exception: Status : 409 - b'<!doctype html><html lang="en"><head><title>HTTP Status 409 \xe2\x80\x93 Conflict</title><style type="text/css">body {font-family:Tahoma,Arial,sans-serif;} h1, h2, h3, b {color:white;background-color:#525D76;} h1 {font-size:22px;} h2 {font-size:16px;} h3 {font-size:14px;} p {font-size:12px;} a {color:black;} .line {height:1px;background-color:#525D76;border:none;}</style></head><body><h1>HTTP Status 409 \xe2\x80\x93 Conflict</h1><hr class="line" /><p><b>Type</b> Status Report</p><p><b>Message</b> Workspace &#39;QQQQQ&#39; already exists</p><p><b>Description</b> The request could not be completed due to a conflict with the current state of the target resource.</p><hr class="line" /><h3>Apache Tomcat/9.0.68</h3></body></html>'

Now in order to capture status code and message from structure html I have to do some text processing.

I was wondering if we would just throw GeoserverException we could have got status_code and could parse html message easily.

Well it is still possible to achieve by preprocessing the exception message.

iamtekson commented 1 year ago

Hi @Wrufesh, Can you please add the actual error message as well? I want to compare it with the pre-processed exception.

Wrufesh commented 1 year ago

Hi @Wrufesh, Can you please add the actual error message as well? I want to compare it with the pre-processed exception.

Sorry for confusing you. I mean the following:

Instead of the following code:

def create_workspace(self, workspace: str):
    """
    Create a new workspace in geoserver.

    The geoserver workspace url will be same as the name of the workspace.
    """
    try:
        url = "{}/rest/workspaces".format(self.service_url)
        data = "<workspace><name>{}</name></workspace>".format(workspace)
        headers = {"content-type": "text/xml"}
        r = self._requests("post", url, data=data, headers=headers)

        if r.status_code == 201:
            return "{} Workspace {} created!".format(r.status_code, workspace)
        else:
            raise GeoserverException(r.status_code, r.content)

    except Exception as e:
        raise Exception(e)

Can we write just:

def create_workspace(self, workspace: str):
    url = "{}/rest/workspaces".format(self.service_url)
    data = "<workspace><name>{}</name></workspace>".format(workspace)
    headers = {"content-type": "text/xml"}
    r = self._requests("post", url, data=data, headers=headers)

    if r.status_code == 201:
        return "{} Workspace {} created!".format(r.status_code, workspace)
    else:
        raise GeoserverException(r.status_code, r.content)
iamtekson commented 1 year ago

You mean to remove try, except block right? Sounds good to me. Please feel free to send PR.

szokejokepu commented 1 year ago

Hi @iamtekson I created a PR for this issue. If possible, after you except you should do a public release, a new patch, because the current build, that you can download with pip is failing on some requests, which are already fixed on the master branch.

iamtekson commented 1 year ago

Hi @szokejokepu, Thanks for the PR. I will review it soon.