search

cognitect-labs / vase

Data driven microservices
Eclipse Public License 1.0
373 stars 42 forks source link

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

Closed mvid closed 5 years ago

mvid commented 6 years ago

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 commented 6 years ago

Thank you for reporting this!

mvid commented 5 years ago

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

ddeaguiar commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

Ok, thanks for following up