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

Configuration for PG Schema Other Than Public #774

Closed
ylg opened this issue Dec 16, 2022 · 9 comments
Closed

Configuration for PG Schema Other Than Public #774

ylg opened this issue Dec 16, 2022 · 9 comments

Comments

@ylg
Copy link

ylg commented Dec 16, 2022

tldr; Is there a specific way to setup for GoodJob tables not in Postgres public, or should it just work?

I imagine I'm making a rookie mistake here, as we've used not used ActiveRecord in any project since the late 2000s. Only pulling it in to support GoodJob (a testament to GoodJob's goodness as AR is reviled here). Anyway, when GoodJob schema objects are not in PG's public schema, GoodJob (AR) seems unable to find them. Normally we set schema search paths on the DB connection URL, but that does not seem to work here. I also tried what seems to be the Rails/AR "way" of using a YAML config, like so:

development: &default
  adapter: postgresql
  host: localhost
  database: some_db
  username: some_user
  password: some_pass
  schema_search_path: 'other_schema,public'

  [...]

But no joy for GoodJob--though a very quick HelloWorld AR model seemed OK. Perhaps GoodJob is using it's own connection logic that doesn't utilize search_path. But, again, my first guess would be a mistake I've made, so I mainly want to confirm I should keep looking, i.e., this a fight with AR not with GJ.

@bensheldon
Copy link
Owner

bensheldon commented Dec 16, 2022

@ylg my intention is that it should just work. Sounds like that isn't the case 😞

I tried changing some settings on the test app (spec/test_app) in this repo. I changed the database.yml schema_search_path to be foobar,public and I do see that in the connection being used:

[6] pry(main)> GoodJob::Execution.connection.exec_query("SHOW search_path").to_a
  SQL (0.8ms)  SHOW search_path
=> [{"search_path"=>"foobar, public"}]

My understanding of Postgres schemas (I could be wrong!) is that when querying postgres with a table name, it will try to look up that table name in those schemas in descending order. That's weird it isn't working!

To be more intentional with overriding the models/queries...

It seems like table_name_prefix has some trouble working with engines: rails/rails#21476

...so you might have to directly crack open the models and update the tables like so:

# initializers/good_job.rb

ActiveSupport::Reloader.to_prepare do
  class GoodJob::Execution
    self.table_name = "foobar.good_jobs"
  end

  class GoodJob::Process
    self.table_name = "foobar.good_job_processes"
  end

  class GoodJob::Setting
    self.table_name = "foobar.good_job_settings"
  end
end

Which is pretty bad, I admit 😞

Edit: I removed the code to also overwrite GoodJob::Job's table name because further discussion surfaced that it was unnecessary and delegated already to GoodJob::Execution.

@ylg
Copy link
Author

ylg commented Dec 16, 2022

Thank you. You're correct on PG's search behavior and we rely on that in quite a few systems, including those using Sequel for Ruby (which is our goto for Rails::API projects like the one I'm trying GoodJob in).

I'll give your self.table_name idea a whirl, maybe with ENV to bring in the schema name as a quick hack, i.e., for foobar in your example. Thanks again!

@ylg
Copy link
Author

ylg commented Dec 16, 2022

Unfortunately, GoodJob itself blocks that with this code in job.rb

      def table_name=(_value)
        raise NotImplementedError, 'Assign GoodJob::Execution.table_name directly'
      end

Presumably there's some other way we're intended to manipulate table_name for GJ?

@bensheldon
Copy link
Owner

@ylg oh, that's a bug! At one time Job inherited from Execution, but no longer. That method override can be removed from Job. Can you make a PR for that?

@ylg
Copy link
Author

ylg commented Dec 16, 2022

On second glance, there is a little more to it (the getter is implemented and is redirected to Execution):

    class << self
      delegate :table_name, to: GoodJob::Execution

      def table_name=(_value)
        raise NotImplementedError, 'Assign GoodJob::Execution.table_name directly'
      end
    end

Perhaps that should be left as, since Execution and Job both seem to want the same table_name? Put another way, is there a scenario where Job.table_name != Execution.table_name is valid? If not, maybe the only issue was attempting to set Job.table_name above?

@ylg
Copy link
Author

ylg commented Dec 16, 2022

By the way, the approach you laid out above (#774 (comment)) works great, just the issue of what to do about setting both Job and Execution, or just one.

@bensheldon
Copy link
Owner

Oops. I didn't see your follow up. You're correct that they should point at the same table. Execution is used for executing jobs, Job is only used as a for the Dashboard ... both use the same underlying table.

@ylg
Copy link
Author

ylg commented Dec 16, 2022

OK, your current implementation seems best to me then, i.e., rather than removing that guard, instead one should just not attempt to set Job.table_name at all.

I mean the whole thing is probably a vanishingly rare scenario to begin with. :)

(If you'd still like a PR just reopen and let me know, happy to help)

@ylg ylg closed this as completed Dec 16, 2022
@bensheldon
Copy link
Owner

@ylg all good! I edited the code snippet I shared to remove the suggestion to overwrite GoodJob::Job.table_name=. Thank you for working through this with me 😓

I'm still curious why the schema-path doesn't work, but unblocking you is the important thing 👍🏻

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

No branches or pull requests

2 participants