Update Framework to Rails 7.2 by awilfox · Pull Request #28 · BerkeleyLibrary/framework

awilfox · 2026-02-21T01:26:29Z

Rails.application.secrets is removed; the secret_key_base accessor in Rails.application has been present since 6.x, so just use that.
Minor configuration updates for 7.2 defaults.
Add mutex_m to avoid deprecation warnings for Ruby 3.4.

Ref: AP-270

This includes the SECRET_KEY_BASE setting in build.yml that @danschmidt5189 suggested in Slack, so cc'ing him.

Only other thing of note is that Rails 7.2 enforces HTTPS (HSTS / redirects) in prod, but this shouldn't be an issue since AFAIK all Library endpoints are HTTPS anyway.

danschmidt5189

A few minor questions but definitely not blockers.

danschmidt5189 · 2026-02-21T02:05:13Z

config/environments/production.rb


  # Force all access to the app over SSL, use Strict-Transport-Security, and use secure cookies.
-  # config.force_ssl = true
+  config.force_ssl = true


Do you know how this behaves behind a proxy that terminates SSL? X-Forwarded-Proto would be https, but the actual connection to the proxy is http.

AVPlayer and Lost-and-Found both have config.force_ssl enabled, so it should be fine here AFAIK.

danschmidt5189 · 2026-02-21T02:07:09Z

docker-compose.ci.yml

      - selenium
    volumes: !override
      - artifacts:/opt/app/artifacts
+      - /tmp/secret_key_base:/run/secrets/SECRET_KEY_BASE


This works. Alternatively you might consider using an actual Docker secret (with file: /tmp/... in the top-level definition).

Copilot

Pull request overview

Updates the application to Rails 7.2, aligning configuration defaults and dependency versions while addressing deprecations (notably Rails.application.secrets removal and Ruby 3.4 warnings).

Changes:

Upgrade Rails from 7.1 to 7.2 (and update Gemfile/Gemfile.lock accordingly, including adding mutex_m).
Update environment and framework configuration defaults (e.g., load_defaults 7.2, SSL enforcement in production, test/development config tweaks).
Adjust CI and tooling scripts (GitHub Actions build workflow, docker-compose CI overrides, simplified binstubs).

Reviewed changes

Copilot reviewed 11 out of 15 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
public/robots.txt	Updates documentation URL to HTTPS.
docker-compose.ci.yml	Adds a CI mount intended to provide `SECRET_KEY_BASE`.
config/puma.rb	Replaces Puma config comments and adds `pidfile` support.
config/environments/test.rb	Enables deprecation logging to stderr; comment updates.
config/environments/production.rb	Enables `force_ssl` and adds Rails 7.2 production defaults.
config/environments/development.rb	Comment/style tweaks; adds Action Mailer template caching setting.
config/boot.rb	Removes explicit `require 'logger'`.
config/application.rb	Switches `load_defaults` to 7.2 and adds `autoload_lib` ignore list.
bin/rubocop	Replaces Bundler-generated binstub with a simplified wrapper and forces explicit config path.
bin/brakeman	Replaces Bundler-generated binstub with a simplified wrapper and adds `--ensure-latest`.
app/models/efees_invoice.rb	Migrates from `Rails.application.secrets` to `Rails.application.secret_key_base`.
Gemfile.lock	Locks Rails 7.2.3 and related dependency updates (including `mutex_m`).
Gemfile	Bumps Rails requirement to 7.2.3 and adds `mutex_m`.
.idea/altmedia.iml	Updates IDE module metadata for new gem versions.
.github/workflows/build.yml	Generates a secret key base file during CI stack setup.

Files not reviewed (1)

.idea/altmedia.iml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-21T02:14:35Z

config/puma.rb

-# Specifies the `environment` that Puma will run in.
+# The ideal number of threads per worker depends both on how much time the
+# application spends waiting for IO operations and on how much you wish to
+# to prioritize throughput over latency.


Typo in comment: duplicated word “to” in “how much you wish to to prioritize…”.

Suggested change

# to prioritize throughput over latency.

# prioritize throughput over latency.

This was part of the Rails default config; I'll fix it, but it's not my typo.

Copilot · 2026-02-21T02:14:36Z

config/puma.rb

+# The default is set to 3 threads as it's deemed a decent compromise between
+# throughput and latency for the average Rails application.
 #
-# preload_app!
+# Any libraries that use a connection pool or another resource pool should
+# be configured to provide at least as many connections as the number of
+# threads. This includes Active Record's `pool` parameter in `database.yml`.
+threads_count = ENV.fetch('RAILS_MAX_THREADS', 5)
+threads threads_count, threads_count


The comment says “The default is set to 3 threads” but ENV.fetch('RAILS_MAX_THREADS', 5) defaults to 5. Update the comment or the default value so they match to avoid misleading operators tuning concurrency.

Fair enough; I kept the thread count we already used for Framework and didn't notice the comment.

Copilot · 2026-02-21T02:14:36Z

docker-compose.ci.yml

    volumes: !override
      - artifacts:/opt/app/artifacts
+      - /tmp/secret_key_base:/run/secrets/SECRET_KEY_BASE


Mounting /tmp/secret_key_base at /run/secrets/SECRET_KEY_BASE won’t affect Rails unless something reads that file and sets ENV["SECRET_KEY_BASE"] (no such logic exists in this repo). If the intent is to satisfy Rails 7.2’s secret_key_base requirement in CI, pass SECRET_KEY_BASE as an environment variable to the container (and/or add boot-time code to read /run/secrets/SECRET_KEY_BASE).

Wrong -- the berkeley_library-docker gem automatically loads secrets from /run/secrets into ENV when the application boots.

Copilot · 2026-02-21T02:14:36Z

.github/workflows/build.yml


      - name: Setup the stack
        run: |
+          docker run "${DOCKER_APP_IMAGE}" rails secret > /tmp/secret_key_base


docker run "${DOCKER_APP_IMAGE}" rails secret should use --rm to avoid leaving stopped containers on the runner. Also, the generated secret is written to /tmp/secret_key_base but isn’t exported as SECRET_KEY_BASE for the compose services (mounting the file alone won’t be used by Rails unless you explicitly read it).

Suggested change

docker run "${DOCKER_APP_IMAGE}" rails secret > /tmp/secret_key_base

SECRET_KEY_BASE="$(docker run --rm "${DOCKER_APP_IMAGE}" rails secret)"

export SECRET_KEY_BASE

printf '%s' "${SECRET_KEY_BASE}" > /tmp/secret_key_base

--rm is fair, but not going to set an env var in favour of the secrets testing.

* `Rails.application.secrets` is removed; the `secret_key_base` accessor in `Rails.application` has been present since 6.x, so just use that. * Minor configuration updates for 7.2 defaults. * Add `mutex_m` to avoid deprecation warnings for Ruby 3.4. Ref: AP-270

awilfox · 2026-02-21T02:30:04Z

v2: Addressed feedback.

awilfox requested review from anarchivist, danschmidt5189, davezuckerman, jason-raitz and yzhoubk February 21, 2026 01:26

danschmidt5189 approved these changes Feb 21, 2026

View reviewed changes

danschmidt5189 requested review from Copilot and removed request for anarchivist, davezuckerman, jason-raitz and yzhoubk February 21, 2026 02:07

Copilot started reviewing on behalf of danschmidt5189 February 21, 2026 02:08 View session

Copilot AI reviewed Feb 21, 2026

View reviewed changes

awilfox force-pushed the awilfox/AP-270-rails-7-2 branch from e66223e to 0decf6b Compare February 21, 2026 02:29

	# to prioritize throughput over latency.
	# prioritize throughput over latency.

-          docker run "${DOCKER_APP_IMAGE}" rails secret > /tmp/secret_key_base
+          SECRET_KEY_BASE="$(docker run --rm "${DOCKER_APP_IMAGE}" rails secret)"
+          export SECRET_KEY_BASE
+          printf '%s' "${SECRET_KEY_BASE}" > /tmp/secret_key_base

Comments

Conversation

awilfox commented Feb 21, 2026

Uh oh!

danschmidt5189 left a comment

Choose a reason for hiding this comment

Uh oh!

danschmidt5189 Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

awilfox Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

danschmidt5189 Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

awilfox Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

awilfox Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

danschmidt5189 Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

awilfox Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

awilfox commented Feb 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants