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

pyramid_openapi3 messes up routes when the server prefix matches #146

Open
Wim-De-Clercq opened this issue Jun 29, 2021 · 5 comments
Open

Comments

@Wim-De-Clercq
Copy link
Contributor

Wim-De-Clercq commented Jun 29, 2021

openapi.yaml:

servers:
  - url: /v
paths:
  /version:
    get:
      ...

pyramid config:

config.add_route("version", "/version")

(along with this config to deal with the prefix on the server)

[filter:paste_prefix]
use = egg:PasteDeploy#prefix
prefix = /v

This code

def remove_prefixes(path):
path = f"/{path}" if not path.startswith("/") else path
for prefix in prefixes:
if path.startswith(prefix):
prefix_length = len(prefix)
return path[prefix_length:]
return path

Will remove the /v from /version and then complain that route "/version" is missing even though it is clearly present in the code. Are prefixes ever part of the route that they need to be removed?

@zupo
Copy link
Collaborator

zupo commented Jun 29, 2021

Huh, I've personally never used paste_prefix before so don't have any intuition into what could be wrong.

Can you provide a minimal runnable example that exercises this bug? Ideally as a fork of this repo and add it to the examples/ folder, so we can run it in CI from now on and make sure prefixes keep working.

@Wim-De-Clercq
Copy link
Contributor Author

Wim-De-Clercq commented Jun 29, 2021

I've made an example here #147

You'll see it's a bit different because I don't know how else to replicate a proper pserve startup with the from wsgiref.simple_server import make_server code from the other example apps.
(/app and /applications as a route was probably a little more realistic than just /v and /version... anyway.)
Looking at https://swagger.io/docs/specification/api-host-and-base-path/

Examples of the url are

servers:
  - url: https://api.example.com/v1 
  - url: https://api.example.com/v1
    description: Production server (uses live data)
  - url: https://sandbox-api.example.com:8443/v1
    description: Sandbox server (uses test data)
  - url: https://{customerId}.saas-app.com:{port}/v2
  - url: /v2

Surely nobody is writing that in the routes. So is there a need to remove them ever?
In my opinion the fix to this issue would be to just remove the code that removes the prefixes. I don't really see a scenario where the routes would contain those server urls.
However, the code is not written without a reason. Maybe this setup creates different routes than other ones?
(original ticket #97)

Edit: Well I guess it's possible that people actually write /v1/ etc as part of the routes and then update all the routes for every version... Maybe an extra check should be added for the slash after the prefix then.

@zupo
Copy link
Collaborator

zupo commented Jun 30, 2021

Surely nobody is writing that in the routes.

What exactly is that in this sentence?

@Wim-De-Clercq
Copy link
Contributor Author

Surely nobody is writing that in the routes.

What exactly is that in this sentence?

Any of the url examples from openapi.yaml.

@zupo
Copy link
Collaborator

zupo commented Jun 30, 2021

I often use the following:

servers:
  - url: /api

See https://github.com/niteoweb/pyramid-realworld-example-app/blob/master/src/conduit/openapi.yaml#L625

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

No branches or pull requests

2 participants