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.

Sign up for GitHub

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

Jump to bottom

Fix the mountable option parser to throw an error if the entrypoint option is not provided instead of falling back to the default value because it's confusing #991

Merged
merged 2 commits into from
Jun 25, 2024
Merged

Fix the mountable option parser to throw an error if the entrypoint option is not provided instead of falling back to the default value because it's confusing #991

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fa6bd06
Fix the mountable option parser to throw an error if the entrypoint o…
whitphx Jun 25, 2024
2293539
Fix
whitphx Jun 25, 2024
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
19 changes: 15 additions & 4 deletions packages/mountable/src/options.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,15 @@ describe("parseMountOptions()", () => {
});
});

it("fills `entrypoint`, and `requirements` fields and converts the `files` into the canonical form", () => {
it("throws an error if the `entrypoint` option is not provided", () => {
expect(() => parseMountOptions({})).toThrowError(
"The `entrypoint` field is required.",
);
});

it("fills the `requirements` field and converts the `files` into the canonical form", () => {
const { kernelOptions } = parseMountOptions({
entrypoint: "streamlit_app.py",
files: {
"streamlit_app.py": "foo",
"foo.txt": {
Expand Down Expand Up @@ -101,6 +108,7 @@ describe("parseMountOptions()", () => {

it("normalizes the archives field", () => {
const { kernelOptions } = parseMountOptions({
entrypoint: "app.py",
archives: [
{
url: "./foo.zip",
Expand All @@ -120,7 +128,7 @@ describe("parseMountOptions()", () => {
],
});
expect(kernelOptions).toEqual({
entrypoint: "streamlit_app.py",
entrypoint: "app.py",
requirements: [],
prebuiltPackageNames: [],
files: {},
Expand Down Expand Up @@ -167,11 +175,12 @@ describe("parseMountOptions()", () => {
it("preserves the `requirements` option if specified", () => {
const { kernelOptions } = parseMountOptions({
requirements: ["matplotlib"],
entrypoint: "app.py",
});
expect(kernelOptions).toEqual({
requirements: ["matplotlib"],
prebuiltPackageNames: [],
entrypoint: "streamlit_app.py",
entrypoint: "app.py",
files: {},
archives: [],
});
Expand All @@ -180,11 +189,12 @@ describe("parseMountOptions()", () => {
it("preserves the `prebuiltPackageNames` option if specified", () => {
const { kernelOptions } = parseMountOptions({
prebuiltPackageNames: ["openssl"],
entrypoint: "foo.py",
});
expect(kernelOptions).toEqual({
requirements: [],
prebuiltPackageNames: ["openssl"],
entrypoint: "streamlit_app.py",
entrypoint: "foo.py",
files: {},
archives: [],
});
Expand All @@ -200,6 +210,7 @@ describe("parseMountOptions()", () => {

it("passes the toast callback options", () => {
const { toastCallbackOptions } = parseMountOptions({
entrypoint: "foo.py",
disableProgressToasts: true,
disableErrorToasts: true,
});
Expand Down
33 changes: 21 additions & 12 deletions packages/mountable/src/options.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,23 @@ import type {
} from "@stlite/kernel";
import type { MakeToastKernelCallbacksOptions } from "@stlite/common-react";

export interface SimplifiedStliteKernelOptions {
entrypoint?: string;
requirements?: StliteKernelOptions["requirements"];
prebuiltPackageNames?: StliteKernelOptions["prebuiltPackageNames"];
files?: Record<
// Simplified version of StliteKernelOptions for the mount function.
// This is exposed to non-typed consumers of the mount function,
// so all fields are typed as optional.
export type SimplifiedStliteKernelOptions = Partial<{
entrypoint: string;
requirements: StliteKernelOptions["requirements"];
prebuiltPackageNames: StliteKernelOptions["prebuiltPackageNames"];
files: Record<
string,
EmscriptenFile | EmscriptenFileUrl | EmscriptenFile["data"] // EmscriptenFile["data"] is allowed as a shorthand for convenience.
>;
archives?: StliteKernelOptions["archives"];
hostConfig?: StliteKernelOptions["hostConfigResponse"];
pyodideUrl?: StliteKernelOptions["pyodideUrl"];
streamlitConfig?: StliteKernelOptions["streamlitConfig"];
idbfsMountpoints?: StliteKernelOptions["idbfsMountpoints"];
}
archives: StliteKernelOptions["archives"];
hostConfig: StliteKernelOptions["hostConfigResponse"];
pyodideUrl: StliteKernelOptions["pyodideUrl"];
streamlitConfig: StliteKernelOptions["streamlitConfig"];
idbfsMountpoints: StliteKernelOptions["idbfsMountpoints"];
}>;

function canonicalizeFiles(
files: SimplifiedStliteKernelOptions["files"],
Expand Down Expand Up @@ -111,9 +114,15 @@ export function parseMountOptions(options: MountOptions): {
const files = canonicalizeFiles(options.files);
const archives = canonicalizeArchives(options.archives);

// TODO: Validate the options object using superstruct.
const entrypoint = options.entrypoint;
if (entrypoint == null) {
throw new Error("The `entrypoint` field is required.");
}

return {
kernelOptions: {
entrypoint: options.entrypoint || DEFAULT_ENTRYPOINT,
entrypoint,
files,
archives,
requirements: options.requirements || [],
Expand Down
Loading