Skip to content

Conversation

moseshll
Copy link
Contributor

@moseshll moseshll commented Jun 5, 2025

  • Invoking the app via rackup allows Rack's defaults to clobber the APP_ENV we are trying to set in Puma.
    • Puma's "user defaults" come from Rack and override the less-privileged defaults coming from ENV and elsewhere.
  • Bypassing rackup and using puma for the entrypoint imitates the Holdings API.
  • Enables logging explicitly (copied from Holdings API) in Sinatra subclass.
  • Includes bundle update to address three Rack-related dependabots.
  • Add benchmark to Gemfile since it's going to be removed from stdlib in Ruby 3.5.

This can be tested locally by running e.g. docker compose up -d web and visiting e.g. localhost:4567/v1/attributes; this branch is also running in k8s at e.g. https://rights-api-testing.macc.kubernetes.hathitrust.org/v1/attributes

moseshll added 4 commits June 5, 2025 13:24
- Invoking the app via `rackup` allows Rack's defaults to clobber the APP_ENV we are trying to set in Puma.
  - Puma's "user defaults" come from Rack and override the less-privileged defaults coming from ENV and elsewhere.
- Bypassing `rackup` and using `puma` for the entrypoint imitates the Holdings API.
@moseshll moseshll marked this pull request as ready for review June 5, 2025 19:34
@moseshll moseshll requested a review from mwarin June 5, 2025 19:56
Copy link

@mwarin mwarin left a comment

Choose a reason for hiding this comment

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

Looks fine, just 1 question about the benchmark gem, which does not block approval. APPROVE.

@moseshll moseshll merged commit 5f7bee4 into main Jun 6, 2025
2 checks passed
@moseshll moseshll deleted the ETT-57_dev_env_pollution branch June 6, 2025 15:59
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