Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Python development compatibility flag for tests #2560

Merged
merged 1 commit into from
Aug 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/workerd/io/compatibility-date.c++
Original file line number Diff line number Diff line change
Expand Up @@ -355,4 +355,12 @@ kj::Maybe<PythonSnapshotRelease::Reader> getPythonSnapshotRelease(
return result;
}

kj::String getPythonBundleName(PythonSnapshotRelease::Reader pyodideRelease) {
danlapid marked this conversation as resolved.
Show resolved Hide resolved
if (pyodideRelease.getPyodide() == "dev") {
return kj::str("dev");
}
return kj::str(pyodideRelease.getPyodide(), "_", pyodideRelease.getPyodideRevision(), "_",
pyodideRelease.getBackport());
}

} // namespace workerd
10 changes: 9 additions & 1 deletion src/workerd/io/compatibility-date.capnp
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,8 @@ struct CompatibilityFlags @0x8f8c1b68151b6cef {
pythonWorkers @43 :Bool
$compatEnableFlag("python_workers")
$pythonSnapshotRelease(pyodide = "0.26.0a2", pyodideRevision = "2024-03-01",
packages = "2024-03-01", backport = 0);
packages = "2024-03-01", backport = 0)
$impliedByAfterDate(name = "pythonWorkersDevPyodide", date = "2000-01-01");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure if I'm understanding this bit. If I set my compatibility date to 2024-02-29, does this mean the python_workers_development flag will be set automatically?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe that it says if pythonWorkersDevPyodide is on and the compat date is after 2000-01-01 (aka always) then pythonWorkers should be on too.

# Enables Python Workers. Access to this flag is not restricted, instead bundles containing
# Python modules are restricted in EWC.
#
Expand Down Expand Up @@ -572,4 +573,11 @@ struct CompatibilityFlags @0x8f8c1b68151b6cef {
# would not be cleared until the next time the queue was processed. This behavior leads
# to a situtation where the stream can hang if the consumer stops consuming. When set,
# this flag changes the behavior to clear the queue immediately upon abort.

pythonWorkersDevPyodide @58 :Bool
$compatEnableFlag("python_workers_development")
$pythonSnapshotRelease(pyodide = "dev", pyodideRevision = "dev",
packages = "2024-03-01", backport = 0)
$experimental;
# Enables Python Workers and uses the bundle from the Pyodide source directory directly. For testing only.
}
1 change: 1 addition & 0 deletions src/workerd/io/compatibility-date.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ kj::String currentDateStr();

kj::Maybe<PythonSnapshotRelease::Reader> getPythonSnapshotRelease(
CompatibilityFlags::Reader featureFlags);
kj::String getPythonBundleName(PythonSnapshotRelease::Reader pyodideRelease);

// These values come from src/workerd/io/compatibility-date.capnp
static constexpr uint64_t COMPAT_ENABLE_FLAG_ANNOTATION_ID = 0xb6dabbc87cd1b03eull;
Expand Down
6 changes: 3 additions & 3 deletions src/workerd/server/tests/python/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,14 @@ load("//src/workerd/server/tests/python:py_wd_test.bzl", "py_wd_test")

load("@bazel_skylib//rules:copy_file.bzl", "copy_file")

# pyodide-dev.capnp.bin represents a custom pyodide version "dev" that is generated
# pyodide_dev.capnp.bin represents a custom pyodide version "dev" that is generated
# at build time using the latest contents of the src/pyodide directory.
# This is used to run tests to ensure that they are always run against the latest build of
# the Pyodide bundle.
copy_file(
name = "pyodide-dev.capnp.bin@rule",
name = "pyodide_dev.capnp.bin@rule",
src = "//src/pyodide:pyodide.capnp.bin",
out = "pyodide-bundle-cache/pyodide-dev.capnp.bin"
out = "pyodide-bundle-cache/pyodide_dev.capnp.bin"
)

py_wd_test(
Expand Down
2 changes: 1 addition & 1 deletion src/workerd/server/tests/python/env-param/env.wd-test
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const unitTests :Workerd.Config = (
),
],
compatibilityDate = "2024-01-15",
compatibilityFlags = ["python_workers"],
danlapid marked this conversation as resolved.
Show resolved Hide resolved
compatibilityFlags = ["python_workers_development"],
)
),
],
Expand Down
2 changes: 1 addition & 1 deletion src/workerd/server/tests/python/hello/hello.wd-test
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const unitTests :Workerd.Config = (
(name = "worker.py", pythonModule = embed "worker.py")
],
compatibilityDate = "2024-01-15",
compatibilityFlags = ["python_workers"],
compatibilityFlags = ["python_workers_development"],
)
),
],
Expand Down
2 changes: 1 addition & 1 deletion src/workerd/server/tests/python/import_tests.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ const unitTests :Workerd.Config = (
(name = "{}", pythonRequirement = ""),
],
compatibilityDate = "2024-05-02",
compatibilityFlags = ["python_workers"],
compatibilityFlags = ["python_workers_development"],
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if we can do a find-and-replace on this string to generate the two variants of tests we talked about. I'll fiddle around with the bazel and make a PR.

)
),
],
Expand Down
4 changes: 2 additions & 2 deletions src/workerd/server/tests/python/py_wd_test.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ def py_wd_test(
args = [],
**kwargs
):
data += ["pyodide-dev.capnp.bin@rule"]
args += ["--pyodide-bundle-disk-cache-dir", "$(location pyodide-dev.capnp.bin@rule)/.."]
data += ["pyodide_dev.capnp.bin@rule"]
args += ["--pyodide-bundle-disk-cache-dir", "$(location pyodide_dev.capnp.bin@rule)/.."]

wd_test(
src = src,
Expand Down
2 changes: 1 addition & 1 deletion src/workerd/server/tests/python/random/random.wd-test
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const unitTests :Workerd.Config = (
(name = "worker.py", pythonModule = embed "worker.py")
],
compatibilityDate = "2024-01-15",
compatibilityFlags = ["python_workers"],
compatibilityFlags = ["python_workers_development"],
)
),
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const unitTests :Workerd.Config = (
(name = "subdir/a.py", pythonModule = embed "./subdir/a.py"),
],
compatibilityDate = "2023-12-18",
compatibilityFlags = ["python_workers"],
compatibilityFlags = ["python_workers_development"],
)
),
],
Expand Down
22 changes: 7 additions & 15 deletions src/workerd/server/workerd-api.c++
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,7 @@ kj::Maybe<jsg::ModuleRegistry::ModuleInfo> WorkerdApi::tryCompileModule(jsg::Loc

namespace {
kj::Path getPyodideBundleFileName(kj::StringPtr version) {
return kj::Path(kj::str("pyodide-", version, ".capnp.bin"));
return kj::Path(kj::str("pyodide_", version, ".capnp.bin"));
}

kj::Maybe<kj::Own<const kj::ReadableFile>> getPyodideBundleFile(
Expand Down Expand Up @@ -476,7 +476,7 @@ kj::Maybe<jsg::Bundle::Reader> fetchPyodideBundle(
kj::HttpHeaders headers(table);

kj::String url =
kj::str("https://pyodide.runtime-playground.workers.dev/pyodide-capnp-bin/pyodide-",
kj::str("https://pyodide.runtime-playground.workers.dev/pyodide-capnp-bin/pyodide_",
version, ".capnp.bin");

auto req = client->request(kj::HttpMethod::GET, kj::StringPtr(url), headers);
Expand Down Expand Up @@ -511,19 +511,11 @@ void WorkerdApi::compileModules(jsg::Lock& lockParam,
"The python_workers compatibility flag is required to use Python.");
// Inject Pyodide bundle
if (util::Autogate::isEnabled(util::AutogateKey::PYODIDE_LOAD_EXTERNAL)) {
KJ_IF_SOME(bundle, fetchPyodideBundle(impl->pythonConfig, "dev"_kj)) {
modules->addBuiltinBundle(bundle, kj::none);
} else {
auto pythonRelease = KJ_ASSERT_NONNULL(getPythonSnapshotRelease(featureFlags));
auto pyodide = pythonRelease.getPyodide();
auto pyodideRevision = pythonRelease.getPyodideRevision();
auto backport = pythonRelease.getBackport();

auto version = kj::str(pyodide, "_", pyodideRevision, "_", backport);
auto bundle = KJ_ASSERT_NONNULL(
fetchPyodideBundle(impl->pythonConfig, version), "Failed to get Pyodide bundle");
modules->addBuiltinBundle(bundle, kj::none);
}
auto pythonRelease = KJ_ASSERT_NONNULL(getPythonSnapshotRelease(featureFlags));
auto version = getPythonBundleName(pythonRelease);
auto bundle = KJ_ASSERT_NONNULL(
fetchPyodideBundle(impl->pythonConfig, version), "Failed to get Pyodide bundle");
modules->addBuiltinBundle(bundle, kj::none);
}
// Inject pyodide bootstrap module (TODO: load this from the capnproto bundle?)
{
Expand Down
Loading