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

Rails 6 & Ruby 3 #578

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

SixiS
Copy link
Contributor

@SixiS SixiS commented Jun 22, 2023

Based off the work by @sosolidkk
#577

Just cleaned it up so it's only the actual code changes to allow it to work in Rails 6 & Ruby 3.

Also

  • Fixed the YAML loading so it doesn't break on Psych version changes & with aliases
  • Added update to the persistence methods seeing as Rails 6 warns on update_attributes

@@ -497,7 +497,7 @@
end

user = User.using(:brazil).where(:name => 'User1').first
expect(user.as_json(:except => [:created_at, :updated_at, :id])).to eq('admin' => nil, 'name' => 'User1', 'number' => nil)
expect(user.as_json(:except => [:created_at, :updated_at, :id, :current_shard])).to eq('admin' => nil, 'name' => 'User1', 'number' => nil)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is :currend_shard included in the result of using() now? That's what broke a bunch of specs when trying to upgrade our rails 5.2 to rails 6.0 using this branch

@discordianfish
Copy link

Is suspect there is also an issue with how rails 6 resets test database that leads to the same/similar issue as described in #361. On rails 5.2 this worked fine but on rails 6 it I get ERROR: cannot drop the currently open database when doing rake db:test:load_schema. I trace this down using bugbye and found out that rails properly closes the connection to the database but Octopus::Proxy's drop_database is opening the database before it runs the drop command, leading to this issue. It's still unclear to me how this relates to the changes here but since it was working with rails 5.2 and I haven't found any similar issues for rails 6.0 w/o ar-octopus I believe this is related.

@discordianfish
Copy link

I've traced this further to load_schema ending up calling establish_master_connection here: https://github.com/rails/rails/blob/v6.0.6.1/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb#L33 which is suppose to connect to the 'postgres' database. Stepping through this it also seems to setup everything as expected but it seems like something ar-octopus is overriding leads to the connection being opend to our test db (as indicated in pg_stat_activity table as well as in ActiveRecord::Base.connection.config) which therefor can't be used to dropped the db.

@SixiS @sosolidkk Can you confirm that for you these things are working? I suspect ar-octopus is assuming a code path that changed in rails 6.0 with it's prelimited multi db support.

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.

2 participants