oldmoe / litestack

MIT License
1.02k stars 56 forks source link

Add setter for `root` to Litesupport #80

Closed hschne closed 4 months ago

hschne commented 8 months ago

This (partially) addresses #34.

As discussed in that ticket, Rails per default uses storage as the location for SQLite DBs, so Litestack's default configuration should adhere to that convention (when using Rails). Furthermore, users should be able to set the location of Litestack's DBs via Ruby if desired.

Implementation

  1. The default location used if Rails is defined has been updated to storage. Generator logic has been updated accordingly (e.g. gitignore)
  2. A root setter was added to Litesupport. This allows setting the Litestack root as desired, e.g.
    # initializers/litestack.rb 
    Litesupport.root = './data'

    Environments will be respected (as in #12), so setting the root as above will result in locations such as ./data/development/data.sqlite3. I made a slight alteration to the database.yml template. Environments are inferred automatically, passing them to root is not necessary.

Known Issues

For some unknown reason ActiveJob initialization and queue definition happens before any initialziers are loaded. So any setting of litesupport.root is ignored, with Rails that means queue.sqlite3 is always created in storage/{env}/queue.sqlite3. Works fine for the other DBs however 🤷

I thought the issue might be jobqueue being initialized when Rails loads, but removing the line from Litejob doesn't help:

module Litejob
  def self.included(klass)
    klass.extend(ClassMethods)
    klass.get_jobqueue # < Removing this doesn't solve the issue
  end
...

@oldmoe @RyanVasichko Maybe #67 could resolve this. In any case, any pointers would be appreciated as I'm quite stuck with this.

Follow Ups

As was discussed in #34 it would be neat to configure Litestack mostly via code rather than YMLs. I'd love to improve the current implementation to allow configuration via config objects - that is, after the issue outlined above has been resolved. Something like this reads nice.

Litestack.configure do |config|
  config.root = './data'
  config.env = 'staging'
end

What do you think?

hschne commented 7 months ago

@wolfgangrittner @oldmoe Thanks for the feedback! 😊

What would you like to do about the known problem with ActiveJob? I've been unsuccessful in trying to address that, so if you got any tips...

So any setting of litesupport.root is ignored, with Rails that means queue.sqlite3 is always created in storage/{env}/queue.sqlite3

Users can work around that by setting the path to the queue DB explicitly in application.rb or similar. Still, don't know if this should be shipped as is. Let me know what you think 🙌

oldmoe commented 4 months ago

I will have to reconsider how the root is detected and also potentially implement the #configure method, which looks really nice, this PR actually conflicts with how Rails initializes test databases (which is why I have to pass the env to the detect_root method). I will close this PR for now and will work on figuring out how to streamline configuration and fix the litejob initialization issue