Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for multiple databases #155

Closed
turgs opened this issue Nov 4, 2024 · 5 comments
Closed

Support for multiple databases #155

turgs opened this issue Nov 4, 2024 · 5 comments

Comments

@turgs
Copy link

turgs commented Nov 4, 2024

I'm using Rails 8 multiple databases. For primary I'm using postgres, but for cache and queues I'm using SQLite.

When trying to run migrations for those databases, this gem looks to be intercepting

$ rails db:migrate:cache --trace
bin/rails aborted!
ActiveRecord::StatementInvalid: SQLite3::SQLException: no such table: pg_proc (ActiveRecord::StatementInvalid)
Caused by:
SQLite3::SQLException: no such table: pg_proc (SQLite3::SQLException)
Tasks: TOP => db:schema:dump:errors

For now I've had to remove this gem as I can't figure out what monkey patch I need to do to try working around it. That said, I think it's similar to things mentioned here:
scenic-views/scenic#402

@teoljungberg
Copy link
Owner

FX does not support sqlite, and while I get use-case for caching - I am not interested in having code find what is the database (in a set of mutliples) with a supported adapter.

@MaksJS
Copy link

MaksJS commented Nov 13, 2024

@turgs I have the same configuration / issue, I've monkey-patched it this way:

module Fx
    module Adapters
      module Postgres
        def functions
          return super if connection.adapter_name == 'PostgreSQL'

          []
        end

        def triggers
          return super if connection.adapter_name == 'PostgreSQL'

          []
        end
      end
    end
end

With the release of Rails 8, I think it will be more frequent and should be fixed at the gem level.

@teoljungberg
Copy link
Owner

If someone can come with a failing test setup for the dummy app inside the integration tests I'd look it over.

@MaksJS
Copy link

MaksJS commented Nov 16, 2024

Heres a config/database.yml file example that would fail with Fx:

default: &default
  encoding: unicode
  pool: <%= ENV.fetch('RAILS_MAX_THREADS') { 5 } %>

postgresql: &postgresql
  <<: *default
  adapter: postgresql

sqlite3: &sqlite3
  <<: *default
  adapter: sqlite3
  timeout: 5000

development:
  primary:
    <<: *postgresql
    database: foo_development
  cable:
    <<: *sqlite3
    database: storage/development_cable.sqlite3
    migrations_paths: db/cable_migrate
  cache:
    <<: *sqlite3
    database: storage/development_cache.sqlite3
    migrations_paths: db/cache_migrate
  queue:
    <<: *sqlite3
    database: storage/development_queue.sqlite3
    migrations_paths: db/queue_migrate

When generating a new Fx function, it would go under db/functions and it will try to migrate it on the "queue" sqlite3 database for example.

manuelvanrijn added a commit to manuelvanrijn/fx that referenced this issue Nov 26, 2024
I was implementing solid_cable using SQLite as an adapter and came across this issue, where the schema dumper was trying to dump the functions and triggers for the SQLite solid cable schema. Obviously, this isn't possible.

I've modified the code to hook into the `ActiveRecord::ConnectionAdapters::PostgreSQL::SchemaDumper` instead of the generic `ActiveRecord::SchemaDumper` and added a test

This should also fix teoljungberg#155
manuelvanrijn added a commit to manuelvanrijn/fx that referenced this issue Nov 26, 2024
I was implementing solid_cable using SQLite as an adapter and came across this issue, where the schema dumper was trying to dump the functions and triggers for the SQLite solid cable schema. Obviously, this isn't possible.

I've modified the code to hook into the `ActiveRecord::ConnectionAdapters::PostgreSQL::SchemaDumper` instead of the generic `ActiveRecord::SchemaDumper` and added a test

This should also fix teoljungberg#155
@manuelvanrijn
Copy link

@teoljungberg I've created a PR addressing this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants