Add .configuration to Grape::API (fixes #1906)

[unleashy]

Still work in progress... includes an intentionally failing test for the inheritance of config to mounted APIs, which I am totally unsure of how to implement and would really like some help with :)

Also if the method name is appropriate. I prefer with_configuration(hash) but @myxoh thought of with(configuration: hash).

Todo list:

• inheritance of config on mounted endpoints
• documentation
• changelog

Thanks!

[dblock]

Feels like base_instance.configuration.merge!(config) should work?

[dblock]

Should this method return a new duplicate of base_instance? Or copy of something though and be chainable?

[unleashy]

#merge won't work because base_instance.configuration is a Grape::Util::EndpointConfiguration, which derives from LazyHash which doesn't seem to implement merge like a "normal" hash. I tried implementing merge there but the way LazyHash works is unclear to me so I wasn't able to do it, sorry.

---

And you're totally right, it should return self so that run MyAPI.with_configuration(...) can work normally, my bad. I don't see why it should return a duplicate though.

[myxoh]

Sorry for the delay here. If you want I can help on implementing merge and merge! on LazyHash

[dblock]

I imaged with_configuation to be a block that yields a copy of things. The way this is written is configuring the API, for which in Ruby we have a better paradigm.

subject.configure do |config|
    config[x] = y
end

Feels also that you shouldn't be able to config anything after mounting it.

[unleashy]

Wait, so you can do that right now? .configure I mean, in a normal grape app, like this:

MyApi.configure do |config|
  config[:bread] = 'bread'
end
# ...
run MyApi

and configure[:bread] will be available, returning 'bread' inside the api normally? If so, this issue is solved because that's exactly what I need.

[dblock]

I don't think you can, but I want to be able to. What I am saying is that is a more standard way of doing configuration in Ruby. Check out https://github.com/slack-ruby/slack-ruby-client/blob/master/lib/slack/config.rb for example.

[unleashy]

Ah, that clears it up, thank you! I'll try my hand at it.

[dblock]

I like this, exposes configure with no other api changes. Document it?

[dblock]

@unleashy Let's finish this? See comments above, bring this to a close?

[unleashy]

Sorry for the delay, I have been busy. I'm kinda sad that I couldn't implement config inheritance but it's ok.

[myxoh]

Hi, sorry for the delay on commenting on this PR. Config inheritance is an interesting one but it might have unintended behaviour, as for the time being the configuration was meant to be specific for the API endpoints.
One way I did do inheritance one I thought that was appropiate was

class Parent < Grape::API
  mount Child, with: configuration
end

class Child < Grape::API
  get configuration[:key] do
    'response'
  end
end

class Base <Grape::API
  mount Parent, with: {key: 'value'}
end

[unleashy]

@myxoh yeah I realised I could do that so it's not a big deal actually hehe.

[dblock]

It looks like what we document in README is misleading then because it seems to imply that you are configuring a specific API? Let's explain how it inherits?

@myxoh Anything else you think this PR needs to merge?

[myxoh]

@dblock you are configuring a specific API, what I was mentioning above is simply that you can use the same strategy to waterfall the dependency (i.e. you configure the top API, and then you code the middle top API to send through the configuration) this doesn't happen automatically, unless you are specifically sending the documentation downstream

[myxoh]

Should we just merge this @dblock?

[dblock]

I hit merge.

[dblock]

Thanks for your solid work @unleashy!

[unleashy]

Thank you all 💟