Skip to content

Fix multiple Connection instances in webpy breaking RunAs#12407

Merged
jimchamp merged 2 commits intointernetarchive:masterfrom
cdrini:fix/multiple-site-runas
Apr 22, 2026
Merged

Fix multiple Connection instances in webpy breaking RunAs#12407
jimchamp merged 2 commits intointernetarchive:masterfrom
cdrini:fix/multiple-site-runas

Conversation

@cdrini
Copy link
Copy Markdown
Collaborator

@cdrini cdrini commented Apr 18, 2026

Part of #11918 . Fixes specifically RunAs not working correctly sometimes when triggered from webpy endpoints.

Technical

Testing

Tested locally:

  • ✅ Login works
  • ✅ Logout works
  • ✅ "Log in as user" works
  • ✅ Tested @jimchamp 's case here and it now correctly displays the user in the transaction:
image - [ ] Needs thorough testing on testing.

Screenshot

Stakeholders

@RayBB @jimchamp

Copilot AI review requested due to automatic review settings April 18, 2026 01:41
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses intermittent failures of RunAs when invoked from web.py endpoints by ensuring request-scoped site context uses the existing web.ctx.site (and its underlying connection) instead of creating a separate Site/connection instance.

Changes:

  • Removed the shared setup_site() helper and stopped creating a new Site during web.py request context setup.
  • Updated web.py context setup to set the site ContextVar directly from web.ctx.site.
  • Kept FastAPI behavior creating a Site per request and setting its auth token from the request cookie.

Comment thread openlibrary/utils/request_context.py Outdated
@mekarpeles mekarpeles added the Priority: 1 Do this week, receiving emails, time sensitive, . [managed] label Apr 20, 2026
Copy link
Copy Markdown
Collaborator

@jimchamp jimchamp left a comment

Choose a reason for hiding this comment

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

This is working in testing, but is not setting author_id in the transaction table in local instances.

@jimchamp jimchamp merged commit 1f934f1 into internetarchive:master Apr 22, 2026
3 checks passed
@cdrini cdrini deleted the fix/multiple-site-runas branch April 22, 2026 03:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

On Testing Priority: 1 Do this week, receiving emails, time sensitive, . [managed]

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants