Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Server Base Path Not Applied to Generated Endpoint URLs #112

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

Closed
Tracked by #422
bowenwr opened this issue Jul 30, 2020 · 4 comments
Closed
Tracked by #422

Server Base Path Not Applied to Generated Endpoint URLs #112

bowenwr opened this issue Jul 30, 2020 · 4 comments
Labels
✨ enhancement New feature or improvement 🍭 OpenAPI Compliance Supporting a new bit of the OpenAPI spec

Comments

@bowenwr
Copy link
Contributor

bowenwr commented Jul 30, 2020

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 bowenwr added the 🐞bug Something isn't working label Jul 30, 2020
@dbanty
Copy link
Collaborator

dbanty commented Jul 31, 2020

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 dbanty added ✨ enhancement New feature or improvement and removed 🐞bug Something isn't working labels Jul 31, 2020
@bowenwr
Copy link
Contributor Author

bowenwr commented Jul 31, 2020

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
Copy link
Contributor Author

bowenwr commented Aug 1, 2020

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.

@jpoehnelt
Copy link
Contributor

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

@dbanty dbanty added the 🍭 OpenAPI Compliance Supporting a new bit of the OpenAPI spec label Aug 13, 2023
@openapi-generators openapi-generators locked and limited conversation to collaborators Aug 13, 2023
@dbanty dbanty converted this issue into discussion #833 Aug 13, 2023

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
✨ enhancement New feature or improvement 🍭 OpenAPI Compliance Supporting a new bit of the OpenAPI spec
Projects
None yet
Development

No branches or pull requests

3 participants