Switch from external pyodide autogate to feature flag (#2807)

We want to be able to synchronize this behavior change between upload time and runtime. Autogates don't allow us to do this as far as I understand but feature flags do.
cloudflare · Oct 1, 2024 · 38cbdaf · 38cbdaf
1 parent bd4c6bf
commit 38cbdaf
Show file tree

Hide file tree

Showing 15 changed files with 21 additions and 54 deletions.
diff --git a/samples/pyodide-fastapi/config.capnp b/samples/pyodide-fastapi/config.capnp
@@ -13,10 +13,6 @@ const config :Workerd.Config = (
       service = "main"
     ),
   ],
-
-  autogates = [
-    "workerd-autogate-pyodide-load-external",
-  ]
 );
 
 const mainWorker :Workerd.Worker = (
@@ -32,7 +28,7 @@ const mainWorker :Workerd.Worker = (
     ),
   ],
   compatibilityDate = "2023-12-18",
-  compatibilityFlags = ["python_workers"],
+  compatibilityFlags = ["python_workers", "python_external_bundle"],
   # Learn more about compatibility dates at:
   # https://developers.cloudflare.com/workers/platform/compatibility-dates/
 );
diff --git a/samples/pyodide-langchain/config.capnp b/samples/pyodide-langchain/config.capnp
@@ -13,10 +13,6 @@ const config :Workerd.Config = (
       service = "main"
     ),
   ],
-
-  autogates = [
-    "workerd-autogate-pyodide-load-external",
-  ]
 );
 
 const mainWorker :Workerd.Worker = (
@@ -26,7 +22,7 @@ const mainWorker :Workerd.Worker = (
     (name = "langchain_openai", pythonRequirement = ""),
   ],
   compatibilityDate = "2023-12-18",
-  compatibilityFlags = ["python_workers"],
+  compatibilityFlags = ["python_workers", "python_external_bundle"],
   # Learn more about compatibility dates at:
   # https://developers.cloudflare.com/workers/platform/compatibility-dates/
 );
diff --git a/samples/pyodide-secret/config.capnp b/samples/pyodide-secret/config.capnp
@@ -17,7 +17,6 @@ const config :Workerd.Config = (
     # Pyodide is included as a builtin wasm module so it requires the
     # corresponding autogate flag.
     "workerd-autogate-builtin-wasm-modules",
-    "workerd-autogate-pyodide-load-external",
   ]
 );
 
@@ -26,7 +25,7 @@ const mainWorker :Workerd.Worker = (
     (name = "worker.py", pythonModule = embed "./worker.py"),
   ],
   compatibilityDate = "2023-12-18",
-  compatibilityFlags = ["python_workers"],
+  compatibilityFlags = ["python_workers", "python_external_bundle"],
   bindings = [
     (
       name = "secret",

diff --git a/samples/pyodide/config.capnp b/samples/pyodide/config.capnp
@@ -13,18 +13,14 @@ const config :Workerd.Config = (
       service = "main"
     ),
   ],
-
-  autogates = [
-    "workerd-autogate-pyodide-load-external",
-  ]
 );
 
 const mainWorker :Workerd.Worker = (
   modules = [
     (name = "worker.py", pythonModule = embed "./worker.py"),
   ],
   compatibilityDate = "2023-12-18",
-  compatibilityFlags = ["python_workers"],
+  compatibilityFlags = ["python_workers", "python_external_bundle"],
   # Learn more about compatibility dates at:
   # https://developers.cloudflare.com/workers/platform/compatibility-dates/
 );
diff --git a/samples/repl-server-python/config.capnp b/samples/repl-server-python/config.capnp
@@ -13,18 +13,14 @@ const config :Workerd.Config = (
       service = "main"
     ),
   ],
-
-  autogates = [
-    "workerd-autogate-pyodide-load-external",
-  ]
 );
 
 const mainWorker :Workerd.Worker = (
   modules = [
     (name = "worker.py", pythonModule = embed "./worker.py"),
   ],
   compatibilityDate = "2023-12-18",
-  compatibilityFlags = ["python_workers"],
+  compatibilityFlags = ["python_workers", "python_external_bundle"],
   # Learn more about compatibility dates at:
   # https://developers.cloudflare.com/workers/platform/compatibility-dates/
 );
diff --git a/src/workerd/api/pyodide/pyodide.h b/src/workerd/api/pyodide/pyodide.h
@@ -421,7 +421,7 @@ bool hasPythonModules(capnp::List<server::config::Worker::Module>::Reader module
 template <class Registry>
 void registerPyodideModules(Registry& registry, auto featureFlags) {
   // We add `pyodide:` packages here including python-entrypoint-helper.js.
-  if (!util::Autogate::isEnabled(util::AutogateKey::PYODIDE_LOAD_EXTERNAL)) {
+  if (!featureFlags.getPythonExternalBundle()) {
     registry.addBuiltinBundle(PYODIDE_BUNDLE, kj::none);
   }
   registry.template addBuiltinModule<PackagesTarReader>(
@@ -431,7 +431,7 @@ void registerPyodideModules(Registry& registry, auto featureFlags) {
 kj::Own<jsg::modules::ModuleBundle> getInternalPyodideModuleBundle(auto featureFlags) {
   jsg::modules::ModuleBundle::BuiltinBuilder builder(
       jsg::modules::ModuleBundle::BuiltinBuilder::Type::BUILTIN_ONLY);
-  if (!util::Autogate::isEnabled(util::AutogateKey::PYODIDE_LOAD_EXTERNAL)) {
+  if (!featureFlags.getPythonExternalBundle()) {
     jsg::modules::ModuleBundle::getBuiltInBundleFromCapnp(builder, PYODIDE_BUNDLE);
   }
   return builder.finish();
@@ -440,7 +440,7 @@ kj::Own<jsg::modules::ModuleBundle> getInternalPyodideModuleBundle(auto featureF
 kj::Own<jsg::modules::ModuleBundle> getExternalPyodideModuleBundle(auto featureFlags) {
   jsg::modules::ModuleBundle::BuiltinBuilder builder(
       jsg::modules::ModuleBundle::BuiltinBuilder::Type::BUILTIN);
-  if (!util::Autogate::isEnabled(util::AutogateKey::PYODIDE_LOAD_EXTERNAL)) {
+  if (!featureFlags.getPythonExternalBundle()) {
     jsg::modules::ModuleBundle::getBuiltInBundleFromCapnp(builder, PYODIDE_BUNDLE);
   }
   return builder.finish();

diff --git a/src/workerd/io/compatibility-date.capnp b/src/workerd/io/compatibility-date.capnp
@@ -615,4 +615,11 @@ struct CompatibilityFlags @0x8f8c1b68151b6cef {
   # compatibility flag we arrange to have such promise continuations scheduled to run
   # in the correct IoContext if it is still alive, or dropped on the floor with a warning
   # if the correct IoContext is not still alive.
+  pythonExternalBundle @63 :Bool
+      $compatEnableFlag("python_external_bundle")
+      $experimental;
+  # Temporary flag to load Python from external capnproto bundle loaded at runtime.
+  # We plan to turn this on always quite soon. It would be an autogate but we need to test
+  # our logic both at upload time and at runtime, and this seemed like the easiest way to
+  # make sure we keep things in sync.
 }
diff --git a/src/workerd/server/tests/python/env-param/env.wd-test b/src/workerd/server/tests/python/env-param/env.wd-test
@@ -14,12 +14,8 @@ const unitTests :Workerd.Config = (
           ),
         ],
         compatibilityDate = "2024-01-15",
-        compatibilityFlags = ["python_workers_development"],
+        compatibilityFlags = ["python_workers_development", "python_external_bundle"],
       )
     ),
   ],
-
-  autogates = [
-    "workerd-autogate-pyodide-load-external",
-  ]
 );
diff --git a/src/workerd/server/tests/python/hello/hello.wd-test b/src/workerd/server/tests/python/hello/hello.wd-test
@@ -8,12 +8,8 @@ const unitTests :Workerd.Config = (
           (name = "worker.py", pythonModule = embed "worker.py")
         ],
         compatibilityDate = "2024-01-15",
-        compatibilityFlags = ["python_workers_development"],
+        compatibilityFlags = ["python_workers_development", "python_external_bundle"],
       )
     ),
   ],
-
-  autogates = [
-    "workerd-autogate-pyodide-load-external",
-  ]
 );
diff --git a/src/workerd/server/tests/python/import_tests.bzl b/src/workerd/server/tests/python/import_tests.bzl
@@ -22,13 +22,9 @@ const unitTests :Workerd.Config = (
           (name = "{}", pythonRequirement = ""),
         ],
         compatibilityDate = "2024-05-02",
-        compatibilityFlags = ["python_workers_development"],
+        compatibilityFlags = ["python_workers_development", "python_external_bundle"],
       )
     ),
-  ],
-
-  autogates = [
-    "workerd-autogate-pyodide-load-external",
   ]
 );"""
 

diff --git a/src/workerd/server/tests/python/random/random.wd-test b/src/workerd/server/tests/python/random/random.wd-test
@@ -8,12 +8,8 @@ const unitTests :Workerd.Config = (
           (name = "worker.py", pythonModule = embed "worker.py")
         ],
         compatibilityDate = "2024-01-15",
-        compatibilityFlags = ["python_workers_development"],
+        compatibilityFlags = ["python_workers_development", "python_external_bundle"],
       )
     ),
   ],
-
-  autogates = [
-    "workerd-autogate-pyodide-load-external",
-  ]
 );
diff --git a/src/workerd/server/tests/python/subdirectory/subdirectory.wd-test b/src/workerd/server/tests/python/subdirectory/subdirectory.wd-test
@@ -10,13 +10,9 @@ const unitTests :Workerd.Config = (
           (name = "subdir/a.py", pythonModule = embed "./subdir/a.py"),
         ],
         compatibilityDate = "2023-12-18",
-        compatibilityFlags = ["python_workers_development"],
+        compatibilityFlags = ["python_workers_development", "python_external_bundle"],
       )
     ),
   ],
-
-  autogates = [
-    "workerd-autogate-pyodide-load-external",
-  ]
 );
 
diff --git a/src/workerd/server/workerd-api.c++ b/src/workerd/server/workerd-api.c++
@@ -511,7 +511,7 @@ void WorkerdApi::compileModules(jsg::Lock& lockParam,
       KJ_REQUIRE(featureFlags.getPythonWorkers(),
           "The python_workers compatibility flag is required to use Python.");
       // Inject Pyodide bundle
-      if (util::Autogate::isEnabled(util::AutogateKey::PYODIDE_LOAD_EXTERNAL)) {
+      if (featureFlags.getPythonExternalBundle()) {
         auto pythonRelease = KJ_ASSERT_NONNULL(getPythonSnapshotRelease(featureFlags));
         auto version = getPythonBundleName(pythonRelease);
         auto bundle = KJ_ASSERT_NONNULL(

diff --git a/src/workerd/util/autogate.c++ b/src/workerd/util/autogate.c++
@@ -17,8 +17,6 @@ kj::StringPtr KJ_STRINGIFY(AutogateKey key) {
   switch (key) {
     case AutogateKey::TEST_WORKERD:
       return "test-workerd"_kj;
-    case AutogateKey::PYODIDE_LOAD_EXTERNAL:
-      return "pyodide-load-external"_kj;
     case AutogateKey::NumOfKeys:
       KJ_FAIL_ASSERT("NumOfKeys should not be used in getName");
   }

diff --git a/src/workerd/util/autogate.h b/src/workerd/util/autogate.h
@@ -14,7 +14,6 @@ namespace workerd::util {
 // Workerd-specific list of autogate keys (can also be used in internal repo).
 enum class AutogateKey {
   TEST_WORKERD,
-  PYODIDE_LOAD_EXTERNAL,
   NumOfKeys  // Reserved for iteration.
 };