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

Fix "missing keyword: :enum_type" in migrations #1133

Merged
merged 1 commit into from
Sep 28, 2023

Conversation

phinze
Copy link
Contributor

@phinze phinze commented Sep 27, 2023

What it does

I think this makes migrations work!

The activerecord-postgres_enum gem was updated across a major version bump in #928

The 2.0.0 gem release notes include the change:

BREAKING Renamed enum_name to enum_type because Rails 7 added basic support for enums

Why it is important

I believe this is necessary for db:migrate to work.

Implementation notes

The schema.rb changes look to be cosmetic reordering except the dropping of this line:

enable_extension "pg_stat_statements"

It looks like this is never added via a migration. From some history spelunking I see it was:

  1. Added in ba23291
  2. Removed explicitly in 6b2e1d2
  3. Added back in 8e35c8f

Do we know if we want this included or not? If so it looks like we can enable it in a migration to get it to stick in the schema.

Your bandwidth for additional changes to this PR

Please choose one of the following to help the project maintainers provide the appropriate level of support:

  • I have the time and interest to make additional changes to this PR based on feedback.
  • I am interested in feedback but don't need to make the changes myself.
  • I don't have time or interest in making additional changes to this work.
  • Other or not sure (please describe):

The `activerecord-postgres_enum` gem was updated across a major version bump here:

chicago-tool-library#928

The 2.0.0 gem release notes include the change:

> BREAKING Renamed enum_name to enum_type because Rails 7 added basic support for enums
@jim jim self-requested a review September 27, 2023 23:42
Copy link
Member

@jim jim left a comment

Choose a reason for hiding this comment

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

Nice catch! I don't think we've run the migrations from scratch recently, but it's good to get this fixed up so it doesn't trip anyone else as they get their environments up and running.

As far as the pg_stat_statements extension is concerned, the bin/reset script calls rails db:close_connections which uses the pg_stat_statements view to close any open connections (otherwise, dropping the database fails). I think that's all it's used for. There might be other ways of closing open connections, in which case, we might not need the extension at all.

I know that extension is enabled on Heroku, and I'm suspicious that it originally leaked into the codebase as a result of pulling down production dumps for analysis.

@jim
Copy link
Member

jim commented Sep 28, 2023

Ran this locally and bin/reset works.

@jim jim merged commit 1d441e9 into chicago-tool-library:main Sep 28, 2023
5 checks passed
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