Release 0.9.0

[crepe]

Release 0.9.0

[krainboltgreene]

• A proper divide between the slimist of crepe and the "basic usage case" of crepe. Right now I'm using crepe as a router and I love it for that. That means though that I don't need half the helpers I'm given.

• Better response body/status code handling. Right now the return of the block is the response body and the status code has to be set via helper (status()).

• Improved performance on widget/:id.

• More convenience functionality so I don't have to repeat things like:

  class Accounts < Crepe::API
    resource :accounts do
      # ...
    end
  end

• Change it so Crepe::API functionality is a module instead of an inherited class.

[davidcelis]

Better response body/status code handling. Right now the return of the block is the response body and the status code has to be set via helper (status()).

What would a more ideal case look like?

Improved performance on widget/:id.

Not sure what this means... Do you mean that Crêpe is less performant with a smaller response body?

[krainboltgreene]

Your benchmarks suggest it could be faster:

/wiggles/:id

Framework	Requests	Response Time	Requests/sec
[Sinatra][sinatra]	15305	117.90ms	85.03
[Camping][camping]	15136	121.07ms	84.08
[Hobbit][hobbit]	15039	121.25ms	83.54
[Crepe][crepe]	14861	120.93ms	82.54
[Grape][grape]	14575	124.94ms	80.97

/wiggles/:id/comments

Framework	Requests	Response Time	Requests/sec
[Camping][camping]	13828	131.78ms	76.81
[Hobbit][hobbit]	13543	132.41ms	75.23
[Grape][grape]	13455	133.76ms	74.74
[Crepe][crepe]	13431	132.55ms	74.61
[Rails::Metal][rails-metal]	13018	139.38ms	72.31

[krainboltgreene]

What would a more ideal case look like?

Maybe returning an array? [status, body] or some method's object?

def respond(status, body)
  Response.new(status, body)
end

Resulting in:

post do
  respond :ok, {}
end

# or

post do
  Control::Account::Create.new(params[:accounts]).to_response
end

class Control::Account::Create
  def initialize(params)
    # ..
  end

  def to_response
    [status, serialization]
  end
end

[davidcelis]

def respond(status, body)
  Response.new(status, body)
end

I could see this being nice. But then again, Crepe::Middleware::RestfulStatus should be providing pretty good defaults based on the HTTP method and the content of the request, and there are helpers that basically do the above for other types of responses (error!, unauthorized!, not_acceptable!). The above might be a more palatable interface for some, but it would be as easy as people defining their own helper method:

helpers do
  def respond(status, body)
    status(status) and body
  end
end

In my own experience with Crêpe, I haven't had to do much mucking around with the status codes. So I'm not sure if this sort of thing is necessary to bake into Crêpe. Anybody else have thoughts?

[krainboltgreene]

Yeah, I ended up having to create a helper as well to solve the problem, but really I'm not sure it should be a thing I have to solve :/

If you put these methods in the router level that means the router is determining the status code.

I guess this is a larger request towards reducing the surface interface of crepe for us who are using it as a really good router interface. My routers should never know how to form a link (url_for), or status codes (error!, unauthorized!, etc). Those are the jobs of other machines. My router should know about the request, the response, and the execution.

[stephencelis]

If you put these methods in the router level that means the router is determining the status code.

Hmm, the problem is that Crepe is a full DSL for building an API, including dispatch and response. Each route() block is wrapped with an Endpoint object. That said, I wouldn't be opposed to a method that simplifies things, e.g.:

class Endpoint
  def finish *args
    # extract status, headers, body, apply to response, halt
  end
end

# E.g.,
#   finish :ok, "Hello from Crepe"
#   finish 201, {"X-Special"=>"Yes"}, "Created"
#   finish(*ResponseBuilder.new(env).as_response)

There is ambiguity, however. Does finish require all values to be raw? Does it convert status symbols and header symbol keys? Does it take the raw, final response or something that will be converted to JSON?

[stephencelis]

Your benchmarks suggest it could be faster: [...snip...]

I think @davidcelis was working on getting updated benchmarks as there was some variation.

The routing is predominantly handled by rack-mount, so I'm not sure we can improve speed here for a 0.9.0 release (as we'd need to write out own routing metal, which may not be worth it).

[stephencelis]

A proper divide between the slimist of crepe and the "basic usage case" of crepe. Right now I'm using crepe as a router and I love it for that. That means though that I don't need half the helpers I'm given.

I understand the desire, but Endpoint is pretty slim (and can be bypassed entirely if you mount Rack apps directly in the routing layer). While I could see having Endpoint::Bare and Endpoint::Default, I'm not sure it's a blocker for 0.9.0. Are the extra methods hampering your ability to use Crepe at this time?

More convenience functionality so I don't have to repeat things like:

class Accounts < Crepe::API
  resource :accounts do
    # ...
  end
end

The pattern I've followed has been, e.g.,

class Router < Crepe::API
  mount Accounts => :accounts
end

class Accounts < Crepe::API
  # Don't need to wrap with `resource/namespace` here.
end

Or are you referring to a different kind of duplication? If it's about creating scaffolding like Rails' resource[s] routing methods, I'm on the fence and am not sure it belongs in Crepe-proper.

Change it so Crepe::API functionality is a module instead of an inherited class.

Interesting. Can you give a more concrete example of what the benefit would be here? What kind of objects do you see extending with API and how? API is just a DSL, really, so the fact that it's a class could be hidden entirely, but we left things as exposed classes because there is a benefit to utilizing inheritance to share logic between multiple API instances. I could see modules used to similar effect, but an API does hold onto a certain amount of state and polluting your own objects with API getters/setters and DSL methods could be a bit heavy.

[stephencelis]

Anyway, my short list of things to look into before 0.9.0:

• Audit rendering. Have we covered our bases here? Should we simplify? Some of our "smart" helpers like the pagination module make a lot of assumptions. How easy does Crepe make it to paginate responses, etc.
• Audit request body parsing. Does it work well?
• Audit tests. We could use a restructuring of our tests. Right now there's a split between testing functionality/behavior and testing methods. And our coverage isn't 100% right now.
• Audit Crepe::Params. We modeled it after Rails' strong parameters, which has a nice and simple DSL, but it may be too simple of a model to be useful without causing confusion in certain cases. I wouldn't mind a more explicit parameter validation DSL.
• Audit versioning. Crepe says it's flexible here, but is it? Do multiple versions of an API belong in the same process? (I don't think so.) Let's build some use cases here for different kinds of versioned APIs and see how Crepe stacks up.

[krainboltgreene]

mount Accounts => :accounts

I had no idea you could do this! That solves it for me.

[davidcelis]

Audit rendering. Have we covered our bases here? Should we simplify? Some of our "smart" helpers like the pagination module make a lot of assumptions. How easy does Crepe make it to paginate responses, etc.

Crepe's pagination is fine for simple things, but as soon as I needed to write serializers or presenters, I ended up writing all of my own paginated logic into the serializers themselves and eschewing the pagination renderer.

Audit Crepe::Params. We modeled it after Rails' strong parameters, which has a nice and simple DSL, but it may be too simple of a model to be useful without causing confusion in certain cases. I wouldn't mind a more explicit parameter validation DSL.

My issues with Crepe::Params have, interestingly enough, been with them being frozen (e.g. https://github.com/goodbrews/api/blob/master/app/apis/users_api.rb#L61-L71). I tried various forms of dup and clone to get a params hash I could alter, and everything out of that was frozen. I finally figured out I could to_h it and get a normal hash. On the other hand, I do think that params should be immutable. But it took me a little while to figure out how to get a hash that I could insert new values into.

[stephencelis]

My issues with Crepe::Params have, interestingly enough, been with them being frozen (e.g. https://github.com/goodbrews/api/blob/master/app/apis/users_api.rb#L61-L71). I tried various forms of dup and clone to get a params hash I could alter, and everything out of that was frozen. I finally figured out I could to_h it and get a normal hash. On the other hand, I do think that params should be immutable. But it took me a little while to figure out how to get a hash that I could insert new values into.

That sounds like a bug to me. Any nested hash on #to_h is still going to be frozen. We may want to store the original, un-deeply-frozen hash, first, replacing the frozen one on #initialize_dup and referencing the original one on #to_h.

[davidcelis]

Rendering and request body parsing could have closer looks taken at them at this point. Our tests could definitely use restructuring, and I would surmise we could use an increase of coverage. I'd prioritize the former over the latter, though... I'd probably say we could release a 0.9.0 version without the latter as well.

I plan to start focusing some love on the test suite; reorganizing it and adding tests as I see a need for them.

[stephencelis]

👍 We're getting there.

[stephencelis]

Rendering and request body parsing could have closer looks taken at them at this point.

An update:

Rendering I think it pretty bare-bones and flexible at this point. I removed the pagination module here: dc7d381 (We can think about making a crepe-pagination gem later.)

Request body parsing indeed needs some work. I've documented its deficiencies here: #55

[stephencelis]

Also would love some discussion around versioning, here: #56

[stephencelis]

One more issue warranting discussion: #57

I think that's basically it. Got a lot of good work done on Crepe the past couple weeks and I think we're close to being stable. If anything else is missing from the thread, bring it up! /cc @kainosnoema

[davidcelis]

This might be worth looking into, but maybe it's not really that big of a deal. In Crepe's main file, we require 'active_support/all'. I suspect we don't actually need all of ActiveSupport. Perhaps we should audit our usage of ActiveSupport and require only what we need? That could also help us eliminate it as a dependency altogether if we care to.

[stephencelis]

I added this recently because Active Support doesn't even manage its own dependency loading very well (you can require HashWithIndifferentAccess but if you try to instantiate one with a nested hash, it'll raise an exception because the related core extension isn't loaded even though such functionality depends on it).

Active Support has a small footprint, really, and once loaded (which is really fast) it's not going to add much overhead. I think most Ruby web developers are going to use it anyway.

If we can actually entertain removing the dependency entirely, OK, but I'm not sure it's worth the end result.