Code review comment for lp:~milo/lava-tool/server-rpc-endpoint-fix

Revision history for this message
Antonio Terceiro (terceiro) wrote :

> === modified file 'lava_tool/utils.py'
> --- lava_tool/utils.py 2013-07-26 14:12:33 +0000
> +++ lava_tool/utils.py 2013-07-30 17:42:26 +0000
> @@ -281,7 +281,7 @@
> editor))
>
>
> -def verify_and_create_url(server, endpoint=""):
> +def verify_and_create_url(endpoint):
> """Checks that the provided values make a correct URL.
>
> If the server address does not contain a scheme, by default it will use
> @@ -291,25 +291,22 @@
> :param server: A server URL to verify.
> :return A URL.
> """
> - scheme, netloc, path, params, query, fragment = \
> - urlparse.urlparse(server)
> - if not scheme:
> - scheme = "https"
> - if not netloc:
> - netloc, path = path, ""
> -
> - if not netloc[-1:] == "/":
> - netloc += "/"
> -
> + url = ""
> if endpoint:
> - if endpoint[0] == "/":
> - endpoint = endpoint[1:]
> - if not endpoint[-1:] == "/":
> - endpoint += "/"
> - netloc += endpoint
> -
> - return urlparse.urlunparse(
> - (scheme, netloc, path, params, query, fragment))
> + scheme, netloc, path, params, query, fragment = \
> + urlparse.urlparse(endpoint)
> + if not scheme:
> + scheme = "https"
> + if not netloc:
> + netloc, path = path, ""
> +
> + url = urlparse.urlunparse(
> + (scheme, netloc, path, params, query, fragment))
> +
> + if url[-1:] != "/":
> + url += "/"
> +
> + return url

I don't understand why you need to parse the url just to unparse it a
few lines below. This function seems rather pointless to me.

« Back to merge proposal