Use from workers import env instead of importing from cloudflare:workers#32
Use from workers import env instead of importing from cloudflare:workers#32
from workers import env instead of importing from cloudflare:workers#32Conversation
G4brym
left a comment
There was a problem hiding this comment.
Automated Code Review — APPROVED ✅
Review Scores: 5/5 reviewers approved
Summary
Clean migration from the JS interop import_from_javascript("cloudflare:workers") pattern to the native Python from workers import env API in both the D1 database backend and R2 storage backend. The change is semantically equivalent, reduces code by 5 lines, removes unnecessary instance state, and uses the newer Workers Python API (cloudflare/workerd#5682).
Review Perspectives
- Correctness: ✅ Semantically equivalent —
getattr(env, self.binding)produces the same result asgetattr(cf_workers.env, self.binding). Lazy init guard correctly updated from_import_from_javascriptto_run_sync. - Security: ✅ No security-relevant changes. Binding names come from Django settings (trusted input).
- Performance: ✅ Python's import cache makes repeated
from workers import envcalls negligible. Removing JS interop is a slight improvement. - Code Quality: ✅ More Pythonic, less state, consistent pattern between both files.
- Testing: ✅ Straightforward API migration with equivalent behavior.
Non-blocking notes
- In
r2.py,from workers import envis now outside thetry/except ImportErrorblock, so a missingworkersmodule would give a rawImportErrorinstead ofException("Code not running inside a worker!"). This is fine in practice sinceworkersis always available whenpyodide.ffiis, and theImportErrormessage is clear on its own.
🤖 Automated review by prodboard
…udflare:workers")`
Replace the indirect `import_from_javascript("cloudflare:workers")` pattern
with the direct `from workers import env` import in both the D1 database
backend and R2 storage backend. This uses the newer Workers Python API
(cloudflare/workerd#5682) which is cleaner and avoids the JavaScript
interop layer for accessing environment bindings.
Fixes #28
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…compat date - Remove all _import_from_javascript references from test_storage_errors.py and test_d1_backend.py since this attribute was removed in the workers env import migration - Bump wrangler compatibility_date from 2025-11-25 to 2026-03-08 in all wrangler.jsonc files (tests/servers/r2, templates/d1, templates/durable-objects) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
bf73690 to
add36d4
Compare
Changes in this updateRebased from main and fixed CI failures:
The newer compat date is required for |
Wrangler 4.51.0's bundled workerd runtime only supports compat dates up to 2025-11-25, which predates the `from workers import env` API. Upgrading to wrangler 4.71.0 brings a newer workerd that supports the 2026-03-08 compat date and the direct env import. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
import_from_javascript("cloudflare:workers")+cf_workers.envpattern with directfrom workers import envin the D1 database backend and R2 storage backendimport_from_javascriptimport and caching from both backendsFixes #28
Changes
django_cf/db/backends/d1/base.py:from workers import import_from_javascriptandself.import_from_javascriptfrom__init__self.import_from_javascript("cloudflare:workers")+getattr(cf_workers.env, ...)withfrom workers import env+getattr(env, ...)django_cf/storage/r2.py:self._import_from_javascriptinstance variable_import_from_javascriptto_run_synccloudflare:workerspattern with directfrom workers import envTest plan
🤖 Generated with Claude Code