Skip to content
Draft
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
100 changes: 100 additions & 0 deletions src/deploy/functions/runtimes/python/index.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import { expect } from "chai";
import * as fs from "fs";
import * as os from "os";
import * as path from "path";
import * as sinon from "sinon";

import * as python from ".";
Expand Down Expand Up @@ -34,4 +37,101 @@ describe("PythonDelegate", () => {
expect(delegate.getPythonBinary()).to.equal("python.exe");
});
});

describe("getExpectedPythonVersion", () => {
it("extracts the major.minor version encoded in the runtime name", () => {
expect(python.getExpectedPythonVersion("python310")).to.equal("3.10");
expect(python.getExpectedPythonVersion("python312")).to.equal("3.12");
});
});

describe("getVenvPythonVersion", () => {
let tmpDir: string;

beforeEach(() => {
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "firebase-python-test-"));
});

afterEach(() => {
fs.rmSync(tmpDir, { recursive: true, force: true });
});

it("reads the version from pyvenv.cfg", () => {
fs.mkdirSync(path.join(tmpDir, "venv"));
fs.writeFileSync(
path.join(tmpDir, "venv", "pyvenv.cfg"),
"home = /usr/bin\nversion = 3.10.12\nexecutable = /usr/bin/python3.10\n",
);

expect(python.getVenvPythonVersion(tmpDir, "venv")).to.equal("3.10");
});

it("reads version_info when version is absent", () => {
fs.mkdirSync(path.join(tmpDir, "venv"));
fs.writeFileSync(
path.join(tmpDir, "venv", "pyvenv.cfg"),
"home = /usr/bin\nversion_info = 3.12.1.final.0\n",
);

expect(python.getVenvPythonVersion(tmpDir, "venv")).to.equal("3.12");
});

it("returns undefined when there is no venv", () => {
expect(python.getVenvPythonVersion(tmpDir, "venv")).to.be.undefined;
});
});

describe("buildVersionMismatchMessage", () => {
it("suggests recreating the venv with a versioned python binary by default", () => {
const message = python.buildVersionMismatchMessage("python312", "3.10", "3.12", "darwin");

expect(message).to.match(
/Python version mismatch.*Python 3\.10.*Python 3\.12.*runtime "python312"/s,
);
expect(message).to.include("python3.12 -m venv venv");
expect(message).to.include('"runtime": "python310"');
});

it("suggests the py launcher to recreate the venv on windows", () => {
const message = python.buildVersionMismatchMessage("python312", "3.10", "3.12", "win32");

expect(message).to.include("py -3.12 -m venv venv");
});

it("omits the runtime suggestion when the venv's python version isn't a supported runtime", () => {
const message = python.buildVersionMismatchMessage("python312", "3.9", "3.12", "darwin");

expect(message).to.match(/Python version mismatch.*Python 3\.9.*Python 3\.12/s);
expect(message).not.to.include('"runtime": "python39"');
});
});

describe("modulesDir version mismatch", () => {
let tmpDir: string;
let platformMock: sinon.SinonStub;

beforeEach(() => {
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "firebase-python-test-"));
platformMock = sinon.stub(process, "platform").value("darwin");
});

afterEach(() => {
platformMock.restore();
fs.rmSync(tmpDir, { recursive: true, force: true });
});
Comment thread
IzaakGough marked this conversation as resolved.

it("throws a clear error when the venv's python version doesn't match the runtime", async () => {
fs.mkdirSync(path.join(tmpDir, "venv"));
fs.writeFileSync(
path.join(tmpDir, "venv", "pyvenv.cfg"),
"home = /usr/bin\nversion = 3.10.12\nexecutable = /usr/bin/python3.10\n",
);

const delegate = new python.Delegate(PROJECT_ID, tmpDir, "python312");

await expect(delegate.modulesDir()).to.eventually.be.rejectedWith(
/Python version mismatch.*Python 3\.10.*Python 3\.12.*runtime "python312"/s,
);
});
});
});
84 changes: 84 additions & 0 deletions src/deploy/functions/runtimes/python/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
{ exit: 1 },
);
}
return Promise.resolve(new Delegate(context.projectId, context.sourceDir, runtime));

Check warning on line 41 in src/deploy/functions/runtimes/python/index.ts

View workflow job for this annotation

GitHub Actions / lint (24)

'Delegate' was used before it was defined
}

/**
Expand Down Expand Up @@ -67,6 +67,71 @@
assertExhaustive(runtime, `Unhandled python runtime ${runtime as string}`);
}

/**
* Extract the "major.minor" Python version a runtime name (e.g. "python310") expects.
* Returns undefined for runtime names that don't encode a version (there are none today,
* but this keeps the check from throwing if that ever changes).
*/
export function getExpectedPythonVersion(
runtime: supported.Runtime & supported.RuntimeOf<"python">,
): string | undefined {
const match = /^python(\d)(\d+)$/.exec(runtime);
if (!match) {
return undefined;
}
return `${match[1]}.${match[2]}`;
}

/**
* Read the Python version a virtual environment was created with, from its pyvenv.cfg.
* Returns undefined if the venv (or its version marker) doesn't exist, so callers can
* fall back to other error messages instead of failing on this best-effort check.
*/
export function getVenvPythonVersion(sourceDir: string, venvDir: string): string | undefined {
let contents: string;
try {
contents = fs.readFileSync(path.join(sourceDir, venvDir, "pyvenv.cfg"), "utf8");
} catch {
return undefined;
}
// pyvenv.cfg has included a "version" line (e.g. "version = 3.10.12") since Python 3.3.
// Python 3.11+ additionally writes "version_info", which we also accept.
const match = /^version(?:_info)?\s*=\s*(\d+)\.(\d+)/m.exec(contents);
if (!match) {
return undefined;
}
return `${match[1]}.${match[2]}`;
}

/**
* Build the "Python version mismatch" error message for a venv created with `actual`
* Python version when `runtime` expects `expected`. Exported (and takes `platform`
* explicitly) so it can be unit tested without going through actual process spawning.
*/
export function buildVersionMismatchMessage(
runtime: supported.Runtime & supported.RuntimeOf<"python">,
actual: string,
expected: string,
platform: NodeJS.Platform = process.platform,
): string {
const recreateCmd =
platform === "win32"
? `py -${expected} -m venv ${DEFAULT_VENV_DIR}`
: `python${expected} -m venv ${DEFAULT_VENV_DIR}`;
const message =
`Python version mismatch: the virtual environment at "${DEFAULT_VENV_DIR}" was created with ` +
`Python ${actual}, but this project is configured to use Python ${expected} (runtime "${runtime}"). ` +
`Recreate the virtual environment with '${recreateCmd}'`;
const actualRuntime = `python${actual.replace(".", "")}`;
if (supported.isRuntime(actualRuntime)) {
return (
message +
`, or set "runtime": "${actualRuntime}" in firebase.json to match the Python version already installed.`
);
}
return message + ".";
}

export class Delegate implements runtimes.RuntimeDelegate {
public readonly language = "python";
constructor(
Expand Down Expand Up @@ -114,6 +179,10 @@
});
this._modulesDir = out.trim();
if (this._modulesDir === "") {
const versionMismatch = this.checkVenvPythonVersionMismatch();
if (versionMismatch) {
throw new FirebaseError(versionMismatch);
}
if (stderr.includes("venv") && stderr.includes("activate")) {
throw new FirebaseError(
"Failed to find location of Firebase Functions SDK: Missing virtual environment at venv directory. " +
Expand All @@ -136,6 +205,21 @@
return getPythonBinary(this.runtime);
}

/**
* Compares the Python version the venv was actually created with against the version
* this.runtime expects, so we can surface a clear error instead of the generic
* "Failed to find location of Firebase Functions SDK" message when they disagree.
* Returns undefined when no mismatch is detected (or can't be determined).
*/
private checkVenvPythonVersionMismatch(): string | undefined {
const expected = getExpectedPythonVersion(this.runtime);
const actual = getVenvPythonVersion(this.sourceDir, DEFAULT_VENV_DIR);
if (!expected || !actual || expected === actual) {
return undefined;
}
return buildVersionMismatchMessage(this.runtime, actual, expected);
}

validate(): Promise<void> {
// TODO: make sure firebase-functions is included as a dep
return Promise.resolve();
Expand All @@ -149,7 +233,7 @@
return Promise.resolve();
}

async serveAdmin(port: number, envs: backend.EnvironmentVariables) {

Check warning on line 236 in src/deploy/functions/runtimes/python/index.ts

View workflow job for this annotation

GitHub Actions / lint (24)

Missing return type on function
const modulesDir = await this.modulesDir();
const envWithAdminPort = {
...envs,
Expand Down
Loading