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

Drop Sessions Table and Delete lib/tasks/sessions.rake #859

Merged
merged 3 commits into from
Aug 22, 2024

Conversation

aaronskiba
Copy link
Collaborator

Fixes #723

Changes proposed in this PR:

  • Drop sessions table from the app's db
    • Within config/initializers/session_store.rb, the app has been configured to use cookie_store for quite some time now. The sessions table has not been used since this change has occurred.
  • Delete lib/tasks/sessions.rake
  • To go along with these codebase changes, the cronjob for rake sessions:cleanup can be removed from all environments (production, staging, and sandbox).

Our app has been using `cookie_store` for some time https://github.com/portagenetwork/roadmap/blob/deployment-portage/config/initializers/session_store.rb. As a result, this rake task is no longer needed.

Also, this rake task is currently broken: https://app.rollbar.com/a/ualbertalib/fix/item/dmp_assistant/490
@aaronskiba
Copy link
Collaborator Author

The failing MySQL actions are expected behaviour after running the PR within this migration.

@aaronskiba
Copy link
Collaborator Author

This PR’s migration is meant to simply drop the sessions table:

class DropSessionsTable < ActiveRecord::Migration[6.1]
  def up
    drop_table :sessions
  end

Unfortunately, it is really transforming the db/schema.rb file: 124bc52#diff-cbf8b25d6d853acf58fa9057841f407d29ffe90649c75cf9e3f51b2d6e3aa1d3
Which in turn is breaking the MySQL workflows. Multiple errors like the following are thrown:

ActiveRecord::MismatchedForeignKey: Column `org_id` on table `annotations` does not match column `id` on `orgs`, which has type `bigint unsigned`. To resolve this issue, change the type of the `org_id` column on `annotations` to be :bigint. (For example `t.bigint :org_id`).
18
Original message: Mysql2::Error: Referencing column 'org_id' and referenced column 'id' in foreign key constraint 'fk_rails_aca7521f72' are incompatible.

This error is occurring because the primary keys are being changed from type :integer to type :serial. The primary key type was never specified in some of these older migrations and a Rails upgrade must be responsible for the change in the default type. However, unlike the primary key, the foreign key was in fact specified as type integer.

There must be a difference in how MySQL vs PostgreSQL interpret the serial type. The primary_key :serial <-> foreign_key :integer works with PostgreSQL, but breaks with MySQL.

The db/schema.rb file is auto-generated and not meant to be physically altered. I had tested out migrations in the past and always encountered this same behaviour. I reached out to the upstream developers. One of them admitted that they had always been physically altering the file to keep the MySQL workflows working.

We discussed it a bit and it sounds like all of the various upstream developers are okay with ditching the MySQL workflows; I guess everyone is using PostgreSQL. The Rails 7 upgrade also currently includes some migrations that would also result in the MySQL workflows breaking.

lagoan
lagoan previously approved these changes Aug 21, 2024
Copy link
Collaborator

@lagoan lagoan left a comment

Choose a reason for hiding this comment

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

Good job Aaron. I like the way you made the sessions migration reversible.

Copy link
Collaborator

@lagoan lagoan left a comment

Choose a reason for hiding this comment

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

LGTM!

@aaronskiba aaronskiba merged commit 4b6e583 into integration Aug 22, 2024
11 of 13 checks passed
@aaronskiba aaronskiba deleted the aaron/issues/723 branch August 22, 2024 17:07
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