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

API description route getting applied over existing route at "/" #91

Closed
mvid opened this issue Apr 4, 2018 · 5 comments
Closed

API description route getting applied over existing route at "/" #91

mvid opened this issue Apr 4, 2018 · 5 comments

Comments

@mvid
Copy link

mvid commented Apr 4, 2018

Description

When one defines an endpoint that handles :get at "/" within an API, it can get replaced by the API descriptor route. This is confusing, as in the Pet Store Full example, there are API endpoints defined at "/". If this is not allowed, it should be documented somewhere, or throw an error. I am happy to submit a pull request when that decision is made.

Expected Behavior

Whatever is defined at "/" works as described in the descriptor file.

Actual Behavior

Either the defined handler or route description handlers get called for a GET request to "/". It seems to pick randomly at server start, and then does not change.

Steps to reproduce

Define a vase api with an endpoint at "/" with a :get. Bring the server up and issue a GET request to the API root, and observe. Bring the server down and back up a few times, and see that the behavior is inconsistent.

Environment

lein run

Operating System (including version).

OSX 10.13.3

Your current Leiningen/Boot/Maven version (lein --version)

Leiningen 2.8.1 on Java 1.8.0_131 Java HotSpot(TM) 64-Bit Server VM

Pedestal and Vase version

[org.clojure/clojure "1.9.0"]
[io.pedestal/pedestal.service "0.5.3"]
[com.cognitect/pedestal.vase "0.9.3"]

@mtnygard
Copy link
Contributor

mtnygard commented Apr 4, 2018

Thank you for reporting this!

@mvid
Copy link
Author

mvid commented Jan 14, 2019

Hi @mtnygard any word on this change? I don't think it is that big, but I did sign the Cognitect agreement

@ddeaguiar ddeaguiar added the 0.9.4 Include in 0.9.4 release label Apr 19, 2019
@ddeaguiar
Copy link
Contributor

In the upcoming Vase release, fern-based api descriptors are preferred. With this type of descriptor, an api descriptor route is not currently created. The inclusion of an api descriptor route should be explicit. Furthermore, the formatting of it should be configurable (See #74).

@ddeaguiar ddeaguiar removed the 0.9.4 Include in 0.9.4 release label May 10, 2019
@ddeaguiar
Copy link
Contributor

After thinking about api description routes further, I've decided to push off support for them in Vase. I'll open a separate issue for that. The existing api description routes were not very useful and Fern-based apis do not include them. The EDN-based api description should be considered legacy and no additional changes will be made to extend them.

@mvid
Copy link
Author

mvid commented May 10, 2019

Ok, thanks for following up

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

3 participants