vigour-io / packer-server

Asset manager for native wrapped application
0 stars 1 forks source link

change `/status` to `/$api/status` and `/$api/config` #30

Closed shawninder closed 8 years ago

shawninder commented 8 years ago

This way apps that expect to serve something called status don't conflict with the /status end-point that packer offers as a way to get the config and state

ninjatux commented 8 years ago

@shawninder we should always stick to path instead of query parameters to provide more compatibility across different tools and a better RESt interface. I would suggest to rename the endpoint to get the config to something more rest like for eaxample: GET /config which explains itself the purpose of the endpoint. In the same way also getting the status should be GET /status.

shawninder commented 8 years ago

@ninjatux What if the app the packer is serving has a file called status (or config)? Then we have a conflict...

shawninder commented 8 years ago

Another idea is to use a path as @ninjatux is suggesting, but differentiate from an asset called status (or config) with a special header.. Not too nice because you can't just check the status or config from your browser... Or maybe a combination of path and query string, like /status?isAsset=false? Feels pretty strange.

I get the impression that something like ?get=status and ?get=config is the best. The packer provides a REST interface for obtaining the app its serving, and a non-REST interface for obtaining META information about the packer itself. Not that it's ideal, but I think it's the best option among those we have thought of up to now...

ninjatux commented 8 years ago

@shawninder and what about a specific path just for packer APIs? eg:

GET /api/status or GET /api/config

shawninder commented 8 years ago

Same problem, the app could have an asset at /api/status. The only safe way I can think of to use a path would be to use a path that is incredibly unlikely to be used by the app, like /packer-server-api/status but that is much less easy to remember...

ninjatux commented 8 years ago

it is very unlikely that an app has an asset in a path like api/status :)

anyway could happen that some customer has this crazy need to have an image called status in his api folder so then I suggest a second way: a TCP api that can be queried on a TCP socket and returns JSON. We can then setup fast status checks over TCP instead of HTTP

shawninder commented 8 years ago

Sounds much harder to user than just opening your browser, visiting the app and fiddling with the url in a really simple way.. Also, I really don't think it's that unlikely for an app to have an /api/status route. What's so important about keeping it RESTful? I don't see any benefit actually.

ninjatux commented 8 years ago

multiple reasons:

shawninder commented 8 years ago

Hmm OK, I didn't realize these things..

So I guess either we

  1. maintain 100% certainty that there won't be collisions with app asset requests
    • the TCP way
    • using a path but requiring a specific HTTP header to differentiate from requests for app assets
  2. forget about 100% certainty
    • using an unlikely path like /packer/status, /api/status, /$/status or something like that

I liked the idea of being able to simply use the browser, which would suggest using option 2, but maybe you have ideas on how to make the TCP or HTTP header option just as easy? Perhaps with a dashboard or something?

I just want to avoid to have to open my terminal just to check which commit hash a packer is serving and I'm afraid of depending on apps not using a certain path because we have absolutely no control over what apps do...

ninjatux commented 8 years ago

sure, TCP way is good for server to server but is a pain for human checks, i agree. so what about:

or something similar?Is nice to define a namespace ($api or $packer for example) and then having the freedom to add more endpoints if needed

shawninder commented 8 years ago

Yes if we use a path I would definitely namespace it. My favourite up to now is /$api.

If you also like this I'd be ready to go ahead with it.

ninjatux commented 8 years ago

@shawninder go go go

shawninder commented 8 years ago

We now have /_api/status and /_api/config