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

Prevent Rails 7.1 create_schema from being added to db/schema.rb #276

Merged

Conversation

patbenatar
Copy link
Contributor

This seems like noise in the schema file for me and I can't see how it'd be t relevant to an Apartment project. In an Apartment project, schemas are managed by Apartment not ActiveRecord like they would be in a vanilla Rails setup.

…chemas are managed by Apartment not ActiveRecord like they would be in a vanilla Rails setup.
@mnovelo
Copy link
Collaborator

mnovelo commented Jul 18, 2024

Yes, we had to do something similar ourselves to stop the schemas from getting added

@mnovelo
Copy link
Collaborator

mnovelo commented Jul 18, 2024

@patbenatar specs are failing with error

NameError:
  uninitialized constant ActiveRecord::ConnectionAdapters::SchemaDumper
  Did you mean?  ActiveRecord::SchemaDumper

@kjakub
Copy link

kjakub commented Sep 18, 2024

I managed to get this working with different approach - (loading issue is probably solvable), but with second look on this approach i asked myself why apartment gem should override another gem - which is active_record default ?

I believe this https://github.com/rails/rails/blob/main/activerecord/lib/active_record/connection_adapters/postgresql/schema_dumper.rb#L31 has a reason to be there. And one would be wondering in future shy create_schema are not appearing in schema.rb.

Imagine scenario where i would be using apartment but not schema/postgresql approach but still using schemas for different reasons.

I would leave this to user, his overrides.

For example in rails, can be initializers/override_schema_dumper.rb

module ActiveRecord
  module ConnectionAdapters
    module PostgreSQL
      class SchemaDumper
        private
        def schemas(stream)
        end
      end
    end
  end
end

And thank you for this gem

@patbenatar
Copy link
Contributor Author

@mnovelo sorry for my delay on this. I'm just getting back to this PR now to see if I can get tests passing. Not sure what the error is all about as this code works in our app, but I'll get to the bottom of it.

@kjakub you bring up a good point that this create_schema suppression is only relevant if you're using schemas with Apartment. I'll see if I can adjust the code to make it only apply the patch when config.use_schemas is set to true.

@patbenatar
Copy link
Contributor Author

@mnovelo ok I believe I've addressed both items in the latest commits. Please re-run CI and let's see if it passes now.

@mnovelo mnovelo merged commit 0c0faf1 into rails-on-services:development Sep 27, 2024
14 of 15 checks passed
@patbenatar
Copy link
Contributor Author

@mnovelo Will you please cut a new release?

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 this pull request may close these issues.

3 participants