killbill / killbill-plugin-framework-ruby

Framework to write Kill Bill plugins in Ruby
http://killbill.io
8 stars 11 forks source link

auto-configuration detection by default #37

Open kares opened 9 years ago

kares commented 9 years ago

looking at commits reminded me that I wanted the plugin's .yml to go through erb (just like in Rails)

@pierre added it already https://github.com/killbill/killbill-plugin-framework-ruby/commit/336b0aa1d1e2905ff6552e645468d980225d10f4 ... so I was thinking about getting it to the 'next' level - generating a default yaml file that auto-detects whether it is running inside KB :

currently on kares-performance branch I'm actually able to use something like :

:stripe:
  :api_secret_key: <%= ENV['API_SECRET_KEY'] %>
  :api_publishable_key: <%= ENV['API_PUBLISHABLE_KEY'] %>
  :test: true

<% if defined? JRUBY_VERSION &&
      (Java::JavaClass.for_name('org.killbill.billing.osgi.bundles.jruby.JRubyPlugin') rescue false) %>
:database:
# In Kill Bill
  :adapter: mysql
  :jndi: 'killbill/osgi/jdbc'
  :pool: false # false-pool (JNDI pool's max)
  # uncomment if pool does not support JDBC4 :
  #:connection_alive_sql: 'select 1'
  # MySQL adapter #configure_connection defaults :
  #    @@SESSION.sql_auto_is_null = 0,
  #    @@SESSION.wait_timeout = 2147483,
  #    @@SESSION.sql_mode = 'STRICT_ALL_TABLES'
  # ... can be disabled (on AR-JDBC 1.4) using :
  :configure_connection: false
<% else %>
:database:
# SQLite (development)
  :adapter: sqlite3
  :database: test.db
# For MySQL
  :adapter: mysql
  :username: root
  :database: 'killbill' # or set the URL :
  :driver: org.mariadb.jdbc.Driver # as in KB
  :pool: 30 # AR's default is max 5 connections
<% end %>

... what this lucks is a simple way of detecting KB - e.g. defined? KILLBILL_SERVER_VERSION or exporting the KillbillServerConfig in a global constant ~ defined? KILLBILL_CONFIG (such a constant could be set before plugins are started) from JRubyPlugin. or one could simply use ENV I'm really not sure about what the "officlal" (best-approach) for the next KB could be ...

p.s. just so I do not forget - tuning erb to work with <%- could be useful as well ...

pierre commented 9 years ago

Some background about configuration:

About this ticket:

kares commented 9 years ago

ah - cool, missed the "sane defaults" - before changing the ticket's concerns I'd like to understand some:

it is unfortunately still required for production deployments, regardless of tenancy

not sure what do you mean by "unfortunately" :) ... you need a place to configure GW credentials or ? ... I also wonder how it would look like - if it's possible to use different credentials per tenant.

for defining a global constant whether we are running inside Kill Bill

OK, thus this can probably go into a separate feature request. also I'm not sure what it should be if you'd like me to do it (seems that exporting the server configuration as KILLBILL_CONFIG might turn our useful for plugins but than it might be confusing if there's a plugin configuration as well - really not sure).


back to database configuration - related to tests. ... if the .yml is to stay what if we used it for test database configuration e.g.

:stripe:
  :api_secret_key: <%= ENV['API_SECRET_KEY'] %>
  :api_publishable_key: <%= ENV['API_PUBLISHABLE_KEY'] %>
  :test: true

<% if ENV['KILLBILL_ENV'] == 'test' %>
:database:
  :adapter: <%= ENV['AR_ADAPTER'] || 'mariadb' %>
  :username: <%= ENV['AR_USERNAME'] || 'killbill' %>
  :database: <%= ENV['AR_DATABASE'] || 'killbill' %>
  :pool: 30 # AR's default is max 5 connections
<% end %>
pierre commented 9 years ago

not sure what do you mean by "unfortunately" :) ... you need a place to configure GW credentials or ? ... I also wonder how it would look like - if it's possible to use different credentials per tenant.

In multi-tenant mode, the latest version of Kill Bill lets you store this YAML file in the database (see the curl example in the Stripe README for instance).

At runtime, we retrieve these per-tenant credentials from Kill Bill, using the tenant id.

OK, thus this can probably go into a separate feature request.

:+1:

also I'm not sure what it should be if you'd like me to do it (seems that exporting the server configuration as KILLBILL_CONFIG might turn our useful for plugins but than it might be confusing if there's a plugin configuration as well - really not sure).

The KillbillServerConfig won't be visible to the plugin (OSGI isolation), and I don't think it should be. Plugins are potentially running untrusted code and we don't want them to have access to the main database credentials for instance (there are actually ways to break this isolation today, but it's the direction we're headed).

I'm actually not sure either what the best way to auto-detect Kill Bill would be. Does JRuby know it is running in a ScriptingContainer? Otherwise detecting JRubyPlugin may be the way to go as you did above (I'm actually surprised it is visible from the plugin, I don't think it's exported explicitly).

  • _spechelper.rb would default to ENV['KILLBILL_ENV'] ||= 'test'

That sounds good (and the sane defaults would be picked up instead when run inside Kill Bill).

  • it still needs to be generated (_spechelper.rb would than read the .yml and use it to connect)

I've actually been learning from the refactoring you did in the specs about some of possibilities with RSpec (I'm still at RSpec 101 :smile:). Wouldn't it be possible to do something similar and have the plugin tests inherit from a shared spec_helper? We already share some test utilities today.

  • driver switching should work - e.g. adapter: postresql will try to load 'jdbc/postres' (just need to make sure they're listed as development gem dependencies)

Actually, that what my main roadblock when I had looked at it a while back. Are all the options passed to ActiveRecord::Base.establish_connection compatible cross drivers?

kares commented 9 years ago

In multi-tenant mode, the latest version of Kill Bill lets you store this YAML file in the database (see the curl example in the Stripe README for instance).

najs!

The KillbillServerConfig won't be visible to the plugin (OSGI isolation), and I don't think it should be. Plugins are potentially running untrusted code and we don't want them to have access to the main database credentials for instance (there are actually ways to break this isolation today, but it's the direction we're headed).

right, makes sense - thanks for clarifying

I'm actually not sure either what the best way to auto-detect Kill Bill would be. Does JRuby know it is running in a ScriptingContainer? Otherwise detecting JRubyPlugin may be the way to go as you did above (I'm actually surprised it is visible from the plugin, I don't think it's exported explicitly).

knowing that we're on ScriptingContainer would be possible via "hacks", and I do not like the current JRubyPlugin way either as it's long and not supposed to work, something "simpler" would be great to have ... opened https://github.com/killbill/killbill-plugin-framework-ruby/issues/39 for tracking the feature

That sounds good (and the sane defaults would be picked up instead when run inside Kill Bill).

yy, unless of course the user explicitly configured something for KB to use ... (e.g. wants to use pool: 20)

Actually, that what my main roadblock when I had looked at it a while back. Are all the options passed to ActiveRecord::Base.establish_connection compatible cross drivers?

yes, except when you use :properties or custom ones the common ones will work across adapters, custom ones e.g. encoding: xxx for MySQL simply gets ignored on SQLite3 ... for test I do not see a major issue here esp. since if there is one we can use ERB to tune the .yml configuration