openapi-python-client icon indicating copy to clipboard operation
openapi-python-client copied to clipboard

Server Base Path Not Applied to Generated Endpoint URLs

Open bowenwr opened this issue 4 years ago • 4 comments

Describe the bug When specifying a url in servers in a spec, the path is completely ignored in the generated endpoints. Making it necessary to fully qualify the base_url passed to the Client, which is problematic when supporting multiple API namespaces (e.g., /api/v2, /api/experimental).

To Reproduce Steps to reproduce the behavior:

  1. Given a spec file with a relative url defined in servers
  2. Generate a client: openapi-python-client generate --path openapi.json
  3. Use the generated client to make a request:
from example_api.client import Client
from example_api.api.requests import get_request

client = Client(base_url="https://example.com/")
response = get_request(client=self.client, request_id="request_id")

The generated code in requests shows:

url = "{}/requests/{request_id}".format(client.base_url, request_id=request_id)

The first part of the URL only puts in the server name passed to Client, but ignores the specified server path from the spec. Resulting in an invalid URL like:

GET https://example.com/requests/request_id

Expected behavior

Should generate an HTTP request like:

GET https://example.com/api/v2/requests/request_id

The generated code would write the base path in like:

url = "{}/api/v2/requests/{request_id}".format(client.base_url, request_id=request_id)

OpenAPI Spec File

{
	"openapi": "3.0.1",
	"info": {
		"title": "Example API",
		"version": "2.0.0"
	},
	"servers": [{
		"url": "/api/v2"
	}],
	"paths": {
		"/requests/{request_id}": {
			"get": {
				"tags": [
					"requests"
				],
				"description": "Get a request by ID",
				"operationId": "getRequest",
				"parameters": [{
					"name": "request_id",
					"in": "path",
					"schema": {
						"type": "string"
					},
					"required": true
				}],
				"responses": {
					"200": {
						"description": "OK",
						"content": {
							"application/json": {
								"schema": {
									"$ref": "#/components/schemas/Request"
								}
							}
						}
					}
				}
			}
		}
	},
	"components": {
		"schemas": {
			"Request": {
				"type": "object",
				"properties": {
					"id": {
						"type": "string"
					}
				}
			}
		}
	}
}

This change might need to be behind a compatibility flag to avoid breaking existing code.

Desktop (please complete the following information):

  • OS: macOS 10.15.2
  • Python Version: 3.8.0
  • openapi-python-client version: 0.4.2

Additional context Add any other context about the problem here.

bowenwr avatar Jul 30 '20 23:07 bowenwr

It will take some consideration add in support for servers, though maybe we can add in partial support in the near term without building out the full set. Here are some things OpenAPI servers are supposed to be able to do:

  1. Provide a list of options, each of which can have some combination of the other features and is optional. So this probably means generating a Server class and some constant instances and taking them as an optional parameter into client.
  2. Support either relative or full URLs.
    1. Full URLs basically replace what is base_url today in the Client. So I guess the server parameter would be required and replace base_url.
    2. Relative URLs (like shown in your example) "indicate that the host location is relative to the location where the OpenAPI document is being served". So, in theory if the client was generated via URL we could build a full URL from the relative one. However if the client is generated from a file, the user of the client would still have to provide a base URL.
  3. Support arbitrary variables (e.g. port, base_path, username)

There are also some usability questions like:

  1. Compatibility- while this project currently makes no promises about backwards compatibility (thus the 0.x versioning), I'd still like to avoid headaches if people upgrade as much as possible.
  2. Ease of use- do we auto-select the first server (like in your example) if the user doesn't opt out? Or only if there's only one server defined?

dbanty avatar Jul 31 '20 14:07 dbanty

Thanks for the response! I definitely appreciate that the servers block could be tricky to fully support. I like the approach/convention of auto-selecting the first server if only one is defined. It seems like the most sane, common behavior.

The client might also be able to store all the generated servers and allow them as optional parameter for a given call. So a sample generated method signature:

Given:

class Client:
	base_url: str
	servers: Optional[List[Server]]

	def default_server(self) -> Optional[Server]:
		if servers:
			return servers[0]
		return None

Sample generated method:

def get_request(*, client: Client, request_id: str, server: Server = client.default_server()) -> Request:

If server is None, base_url is used as-is like the current behavior.

bowenwr avatar Jul 31 '20 19:07 bowenwr

We might also add server_variables: Dict[str, str] to the generated method signature to provide runtime substitutions. Given something like:

@dataclass
class ServerVariable:
    """ A representation of the OpenAPI Server Variable Object"""

    default: str
    description: Optional[str]
    enum: Optional[List[str]]


@dataclass
class Server:
    """ A representation of the OpenAPI Server Object"""

    url: str
    description: Optional[str]
    variables: Optional[Dict[str, ServerVariable]]

    def resolve_url(self, variable_values: Optional[Dict[str, str]]) -> str:
        substitutions = self._default_values().copy()
        substitutions.update(variable_values)
        return self.url.format(**substitutions)

    def _default_values(self) -> Dict[str, str]:
        return {
            key: variable.default
            for key, variable in self.variables.items()
        }

The endpoint internally would call the specified server's server.resolve_url(server_variables) to get a fully formed URL.

bowenwr avatar Aug 01 '20 01:08 bowenwr

Just ran into this, probably a hard blocker for our usage.

jpoehnelt avatar Nov 10 '21 21:11 jpoehnelt