mattetti / Weasel-Diesel

DSL to describe, document and test web services
MIT License
439 stars 21 forks source link

Handle API Versioning #24

Open kamui opened 11 years ago

kamui commented 11 years ago

@mattetti @raul

One of the major issues with API frameworks is how versioning is handled. I was wondering what your thoughts were about dealing with versioning in WD.

As far as I can tell this probably needs a separate issue in wd_sinatra. In WD, we'll need to decide what the DSL will look like for defining/namespacing different versions of services.

In wd-sinatra, we'll need to decide how to implement it and how to detect a specific version. IE.

  1. Accept Header accept-extension: application/json;q=1;v=3
  2. Accept Header custom media type: application/vnd.example.v3+json
  3. URL Path: GET /v1/hello_world
  4. Accept-Version - grape, node-restify use this, it's not part of RFC 2616

I prefer 1, but have heard a decent case for 2 as well.

mattetti commented 11 years ago

I HATE accept headers for versioning, this is for me a big no-no. They are a pain to debug, you don't see anything in the logs and API users get confused. Having the version part of the path is the simplest, easiest way and I guarantee you that it works very well :p

On Fri, May 31, 2013 at 1:04 AM, Jack Chu notifications@github.com wrote:

@mattetti https://github.com/mattetti @raul https://github.com/raul

One of the major issues with API frameworks is how versioning is handled. I was wondering what your thoughts were about dealing with versioning in WD.

As far as I can tell this probably needs a separate issue in wd_sinatra. In WD, we'll need to decide what the DSL will look like for defining/namespacing different versions of services.

In wd-sinatra, we'll need to decide how to implement it and how to detect a specific version. IE.

  1. Accept Header accept-extension: application/json;q=1;v=3
  2. Accept Header custom media type: application/vnd.example.v3+json
  3. URL Path: GET /v1/hello_world
  4. Accept-Version - grape, node-restify use this, it's not part of RFC 2616

I prefer 1, but have heard a decent case for 2 as well.

— Reply to this email directly or view it on GitHubhttps://github.com/mattetti/Weasel-Diesel/issues/24 .

raul commented 11 years ago

I don't have strong feelings on this, but I see many people do. Do you think we can implement this with an opinionated default that users can easily override (so it doesn't get overwritten with every wd_sinatra update)?

mattetti commented 11 years ago

Yes, very simple: we don't add built in support for header versioning :) If people want it, they can add it via a gem On May 31, 2013 7:33 PM, "raul murciano" notifications@github.com wrote:

I don't have strong feelings on this, but I see many people do. Do you think we can implement this with an opinionated default that users can easily override (so it doesn't get overwritten with every wd_sinatra update)?

— Reply to this email directly or view it on GitHubhttps://github.com/mattetti/Weasel-Diesel/issues/24#issuecomment-18759517 .

kamui commented 11 years ago

Ha, I'm on the other side of this. I really don't think versioning belongs in the route. Also, because content type is now set via accept header, you need to deal with it anyhow, so why not also handle versioning with content negotiation? As far as logging, you could always log the accept header, or the pieces you want logged in Sinatra. If API users get confused by the accept header, I don't see how they'll figure out how to set the correct media type.

But aside from the versioning method, don't we need to define how to set a version in the WD DSL? If anything so when you generate documentation, the two versions are split up. So if you use versioning in your routes, you don't have v1-v3 documentation all mixed together. I think if you're using v3, you only want to see v3 documentation.

I kind of think we should handle versioning like grape does it. Support different methods of versioning, which work off of one DSL.

mattetti commented 11 years ago

so why not also handle versioning with content negotiation? Because it's hidden, it's not a convention and I think it's a terrible idea :p

As far as logging, you could always log the accept header I use WD on APIs with really high throughput, we don't log things at the application layer unless we really have to. We are looking at web server logs and ou don't get to parse custom headers so that's not really an option.

I don't see how they'll figure out how to set the correct media type. Media types aren't frequently used in big APIs, most APIs only return a type and that's fine. As a matter of fact when you run a public API, you sees lots of client not even settings these headers. In my mind, unless you decide to enforce them yourselves, they shouldn't be required. KISS

But aside from the versioning method, don't we need to define how to set a version in the WD DSL? Yes, for many years I did it by simply duplicating the service and changing the url. This works beautifully because you aren't changing the existing APIs by accident and you get proper and simpler isolation.

So if you use versioning in your routes, you don't have v1-v3 documentation all mixed together. That seems like a bad argument to me, we are just talking about documentation generation, organizing the documentation based on the url version isn't hard ;) I would also not want to see all the versions of an end point documentation. I would pick the version I use and read the doc :)

I kind of think we should handle versioning like grape does it. I think there is room for a plugin/gem to do that, but I don't want WD to turn into Rails. I really think that we should only add features that are critical.

On Fri, May 31, 2013 at 8:32 PM, Jack Chu notifications@github.com wrote:

Ha, I'm on the other side of this. I really don't think versioning belongs in the route. Also, because content type is now set via accept header, you need to deal with it anyhow, so why not also handle versioning with content negotiation? As far as logging, you could always log the accept header, or the pieces you want logged in Sinatra. If API users get confused by the accept header, I don't see how they'll figure out how to set the correct media type.

But aside from the versioning method, don't we need to define how to set a version in the WD DSL? If anything so when you generate documentation, the two versions are split up. So if you use versioning in your routes, you don't have v1-v3 documentation all mixed together. I think if you're using v3, you only want to see v3 documentation.

I kind of think we should handle versioning like grape does it. Support different methods of versioning, which work off of one DSL.

— Reply to this email directly or view it on GitHubhttps://github.com/mattetti/Weasel-Diesel/issues/24#issuecomment-18763203 .

pheino commented 11 years ago

I'm finding the need to support a version-ed API thru the DSL

I agree about the header thing Matt ;)

Tho I don't like the idea of making a new service just because I hate code duplication of any sort.
I'm most likely going to implement an API like this "/v1/hello_world", but looking for a little help from the DSL

@kumal - If you want the header support just write a rack middleware to parse the header and modify it to set param[:version] ;)

Then define your service spec

describe_service ":version/hello_world" do |service|
  service.formats   :json
  service.http_verb :get

  service.params do |p|
    p.string :version, :required => true, :options => ['v1']
  end
end

Here some other rack version gems http://tomdmaguire.wordpress.com/2011/11/12/creating-a-restful-versioned-api-using-sinatra/

Twitter documentation is by version. :) I think picking your version and then looking at those specific docs is the most logical and expected approach.

@mattetti - do you think using the same service for each version of an API tends to lead to more problems than its solving? I guess it probably depends if your modifying the contract of the API itself whether it warrants a new version of the API.

describe_service ":version/hello_world" do |service|
  service.implementation do
    if params[:version] == 'v1'
    else
    end
  end
end

describe_service "v1/hello_world" do |service|
end

describe_service "v2/hello_world" do |service|
end
mattetti commented 11 years ago

@pheino The reasons why I prefer to split the various versions of an API into different implementations is

Code duplication is only a problem if the logic is duplicated, here we have 2 versions of the logic, therefore it's not a duplication it's a safe branching of the logic. This way you know you don't introduce regressions.

theCrab commented 10 years ago

I like @mattetti's approach.

safarista commented 10 years ago

How do you implement an index list service that doesn't need any params past to it?