Skip to content

fix(proxy): restore 5 items dropped by bad rebase in #12724 - #13063

Merged
mekarpeles merged 1 commit into
masterfrom
12724/fix-bad-rebase-regression
Jun 30, 2026
Merged

fix(proxy): restore 5 items dropped by bad rebase in #12724#13063
mekarpeles merged 1 commit into
masterfrom
12724/fix-bad-rebase-regression

Conversation

@openlibrary-bot

@openlibrary-bot openlibrary-bot commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

Summary

PR #12724 (single http_proxy consolidation) contained a bad rebase that accidentally dropped 5 items from master that had nothing to do with the proxy refactor. This PR surgically restores them.

Triggered by @mekarpeles

What regressed

File Item dropped Impact
conf/openlibrary.yml otp_seed: dev-otp-seed-for-e2e-testing OTP auth fails in dev/e2e
conf/openlibrary.yml sentry.frontend DSN block Sentry JS event reporting broken
conf/openlibrary.yml profiles_sample_rate: 0.05 (changed to 0.001) Sentry profile sampling wrong
conf/openlibrary.yml author_exclusions: [] 404-exclusion author list gone
requirements.txt cryptography==44.0.2 CriticalModuleNotFoundError on any S3 auth cookie path

The cryptography omission is the most critical: openlibrary/accounts/model.py imports from cryptography.fernet import Fernet at runtime for S3-key cookie encryption (added in #12856). Its absence causes a hard ModuleNotFoundError on any code path that touches S3 auth cookies.

How to confirm the regression exists on current master:

docker compose run --rm home python -c "from cryptography.fernet import Fernet"
# ModuleNotFoundError: No module named 'cryptography'

What this PR does

Restores exactly those 5 items — nothing else. The proxy changes from #12724 (vendors.py, affiliate_server.py, http_proxy config comment, python-amazon-paapi fork pin) are preserved.

The net diff against the pre-merge commit (7c92384d3) shows only the two intentional changes from #12724: the http_proxy comment block and the python-amazon-paapi fork pin. All 5 regressions are fully restored.

Verification

# After rebuilding Docker with the restored requirements.txt:
docker compose run --rm home python -c \
  "from cryptography.fernet import Fernet; print('cryptography OK:', Fernet.generate_key()[:20])"
# cryptography OK: b'mLNoHnhGsx2PVXSDqH-K'

docker compose run --rm home python -m pytest \
  openlibrary/tests/core/test_processors.py -x -v
# 9 passed

Related

The rebase on #12724 (single http_proxy consolidation) incorrectly
dropped changes from master that the proxy refactor had no relation to:

- conf/openlibrary.yml: restore otp_seed dev seed (needed by OTP auth)
- conf/openlibrary.yml: restore sentry.frontend DSN block (Sentry JS)
- conf/openlibrary.yml: restore profiles_sample_rate: 0.05 (was 0.001)
- conf/openlibrary.yml: restore author_exclusions: [] (404 exclusions)
- requirements.txt: restore cryptography==44.0.2 (Fernet cookie encrypt)

The cryptography omission is the most critical: accounts/model.py
imports Fernet at runtime for S3 key cookie encryption, so its absence
causes ModuleNotFoundError on any code path touching S3 auth cookies.

Verified:
  docker compose run --rm home python -m pytest \
    openlibrary/tests/core/test_processors.py -x -v
  # 9 passed

  docker compose run --rm home python -c \
    "from cryptography.fernet import Fernet; print('OK')"
  # cryptography OK

Closes #12724 regression
@openlibrary-bot

Copy link
Copy Markdown
Collaborator Author

Thank you for opening this PR!

🤖 Copilot has been assigned for an initial review.

A reviewer must first be assigned. There are no open PRs of equal or higher priority ahead of this one.

PR triage checklist (maintainers / Richy)
  • PR description — not empty; explains what the change does and how to verify it
  • References an issue — PR body contains a #NNN reference
    • Linked issue is triaged — has a Priority: * label (not just Needs: Triage)
    • Linked issue is assigned — has at least one assignee
  • Commit history clean — no WIP/fixup/conflict noise; commit messages are meaningful
  • CI passing — no failing check-runs
  • Test cases present — config and dependency restore; no logic changes requiring test coverage
  • Proof of testing — PR body includes Docker verification commands

Note

This comment was automatically generated by PAM, Open Library's Project AI Manager. PAM provides status visibility, performs basic project management functions, and gives actionable feedback so contributors aren't left waiting.

@mekarpeles mekarpeles self-assigned this Jun 30, 2026
@mekarpeles
mekarpeles merged commit 835af7d into master Jun 30, 2026
8 checks passed
@mekarpeles
mekarpeles deleted the 12724/fix-bad-rebase-regression branch June 30, 2026 04:27
@mekarpeles

Copy link
Copy Markdown
Member

These changes all LGTM cc: @cdrini; I have reviewed and tested and also spun up a second reviewer to verify the results didn't miss anything.

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