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

Fix datadir CLI arg type #4643

Merged
merged 3 commits into from
Oct 5, 2022
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
2 changes: 1 addition & 1 deletion packages/cli/src/cmds/beacon/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export async function beaconHandler(args: IBeaconArgs & IGlobalArgs): Promise<vo
mkdir(beaconPaths.dbDir);

const abortController = new AbortController();
const logger = getCliLogger(args, {defaultLogFile: "beacon.log"}, config);
const logger = getCliLogger(args, {defaultLogFilepath: path.join(beaconPaths.dataDir, "beacon.log")}, config);

onGracefulShutdown(async () => {
abortController.abort();
Expand Down
7 changes: 5 additions & 2 deletions packages/cli/src/cmds/lightclient/handler.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
import path from "node:path";
import {getClient} from "@lodestar/api";
import {Lightclient} from "@lodestar/light-client";
import {fromHexString} from "@chainsafe/ssz";
import {getBeaconConfigFromArgs} from "../../config/beaconParams.js";
import {getGlobalPaths} from "../../paths/global.js";
import {IGlobalArgs} from "../../options/index.js";
import {getCliLogger} from "../../util/index.js";
import {ILightClientArgs} from "./options.js";

export async function lightclientHandler(args: ILightClientArgs & IGlobalArgs): Promise<void> {
const {config} = getBeaconConfigFromArgs(args);
const {config, network} = getBeaconConfigFromArgs(args);
const globalPaths = getGlobalPaths(args, network);

const logger = getCliLogger(args, {defaultLogFile: "lightclient.log"}, config);
const logger = getCliLogger(args, {defaultLogFilepath: path.join(globalPaths.dataDir, "lightclient.log")}, config);
const {beaconApiUrl, checkpointRoot} = args;
const api = getClient({baseUrl: beaconApiUrl}, {config});
const {data: genesisData} = await api.beacon.getGenesis();
Expand Down
3 changes: 2 additions & 1 deletion packages/cli/src/cmds/validator/handler.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import path from "node:path";
import {setMaxListeners} from "node:events";
import {LevelDbController} from "@lodestar/db";
import {ProcessShutdownCallback, SlashingProtection, Validator, ValidatorProposerConfig} from "@lodestar/validator";
Expand Down Expand Up @@ -28,7 +29,7 @@ export async function validatorHandler(args: IValidatorCliArgs & IGlobalArgs): P
const validatorPaths = getValidatorPaths(args, network);
const accountPaths = getAccountPaths(args, network);

const logger = getCliLogger(args, {defaultLogFile: "validator.log"}, config);
const logger = getCliLogger(args, {defaultLogFilepath: path.join(validatorPaths.dataDir, "validator.log")}, config);

const persistedKeysBackend = new PersistedKeysBackend(accountPaths);
const valProposerConfig = getProposerConfigFromArgs(args, {persistedKeysBackend, accountPaths});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import path from "node:path";
import {InterchangeFormatVersion} from "@lodestar/validator";
import {ICliCommand, writeFile600Perm} from "../../../util/index.js";
import {IGlobalArgs} from "../../../options/index.js";
Expand Down Expand Up @@ -39,9 +40,9 @@ export const exportCmd: ICliCommand<

handler: async (args) => {
const {config, network} = getBeaconConfigFromArgs(args);

const validatorPaths = getValidatorPaths(args, network);
// slashingProtection commands are fast so do not require logFile feature
const logger = getCliLogger(args, {defaultLogFile: "validator.log"}, config);
const logger = getCliLogger(args, {defaultLogFilepath: path.join(validatorPaths.dataDir, "validator.log")}, config);

const {validatorsDbDir: dbPath} = getValidatorPaths(args, network);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import fs from "node:fs";
import path from "node:path";
import {Interchange} from "@lodestar/validator";
import {ICliCommand} from "../../../util/index.js";
import {IGlobalArgs} from "../../../options/index.js";
Expand Down Expand Up @@ -40,8 +41,9 @@ export const importCmd: ICliCommand<

handler: async (args) => {
const {config, network} = getBeaconConfigFromArgs(args);
const validatorPaths = getValidatorPaths(args, network);
// slashingProtection commands are fast so do not require logFile feature
const logger = getCliLogger(args, {defaultLogFile: "validator.log"}, config);
const logger = getCliLogger(args, {defaultLogFilepath: path.join(validatorPaths.dataDir, "validator.log")}, config);

const {validatorsDbDir: dbPath} = getValidatorPaths(args, network);

Expand Down
4 changes: 3 additions & 1 deletion packages/cli/src/options/globalOptions.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import {ACTIVE_PRESET} from "@lodestar/params";
import {NetworkName, networkNames} from "../networks/index.js";
import {ICliCommandOptions, readFile} from "../util/index.js";
import {paramsOptions, IParamsArgs} from "./paramsOptions.js";

interface IGlobalSingleArgs {
dataDir: string;
dataDir?: string;
network?: NetworkName;
paramsFile: string;
preset: string;
Expand Down Expand Up @@ -33,6 +34,7 @@ const globalSingleOptions: ICliCommandOptions<IGlobalSingleArgs> = {
preset: {
hidden: true,
type: "string",
default: ACTIVE_PRESET,
},
};

Expand Down
6 changes: 3 additions & 3 deletions packages/cli/src/util/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,12 @@ export interface ILogArgs {
*/
export function getCliLogger(
args: ILogArgs & Pick<IGlobalArgs, "dataDir">,
paths: {defaultLogFile: string},
paths: {defaultLogFilepath: string},
config: IChainForkConfig,
opts?: {hideTimestamp?: boolean}
): ILogger {
const consoleTransport = new ConsoleDynamicLevel({
// Set defaultLevel, not level for dynamic level setting of ConsoleDynamicLevel
// Set defaultLevel, not level for dynamic level setting of ConsoleDynamicLvevel
defaultLevel: args.logLevel ?? LOG_LEVEL_DEFAULT,
debugStdout: true,
handleExceptions: true,
Expand Down Expand Up @@ -72,7 +72,7 @@ export function getCliLogger(
// `lodestar --logFileDailyRotate 10` -> set daily rotate to custom value 10
// `lodestar --logFileDailyRotate 0` -> disable daily rotate and accumulate in same file
const rotateMaxFiles = args.logFileDailyRotate ?? LOG_DAILY_ROTATE_DEFAULT;
const filename = args.logFile ?? path.join(args.dataDir, paths.defaultLogFile);
const filename = args.logFile ?? paths.defaultLogFilepath;
const logFileLevel = args.logFileLevel ?? LOG_FILE_LEVEL_DEFAULT;

transports.push(
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/test/e2e/importKeystoresFromApi.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ describeCliTest("import keystores from api", function ({spawnCli}) {
);

// Attempt to run a second process and expect the keystore lock to throw
const vcProc2 = spawnCli([
const vcProc2 = spawnCli({pipeStdToParent: true, logPrefix: "vc-2"}, [
// ⏎
"validator",
`--dataDir=${dataDir}`,
Expand Down
32 changes: 32 additions & 0 deletions packages/cli/test/e2e/runDevCmd.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import {getClient} from "@lodestar/api";
import {config} from "@lodestar/config/default";
import {retry} from "@lodestar/utils";
import {describeCliTest} from "../utils/childprocRunner.js";
import {itDone} from "../utils/runUtils.js";

describeCliTest("Run dev command", function ({spawnCli}) {
itDone("Run dev command with no --dataDir until beacon api is listening", async function (done) {
const beaconPort = 39011;

const devProc = spawnCli({pipeStdToParent: false, printOnlyOnError: true, logPrefix: "dev"}, [
// ⏎
"dev",
"--reset",
"--startValidators=0..7",
`--rest.port=${beaconPort}`,
]);

// Exit early if process exits
devProc.on("exit", (code) => {
if (code !== null && code > 0) {
done(Error(`process exited with code ${code}`));
}
});

const beaconUrl = `http://localhost:${beaconPort}`;
const client = getClient({baseUrl: beaconUrl}, {config});

// Wrap in retry since the API may not be listening yet
await retry(() => client.node.getHealth(), {retryDelay: 1000, retries: 60});
});
});
2 changes: 1 addition & 1 deletion packages/cli/test/e2e/voluntaryExit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ describeCliTest("voluntaryExit cmd", function ({spawnCli}) {
itDone("Perform a voluntary exit", async function (done) {
const restPort = 9596;

const devBnProc = spawnCli([
const devBnProc = spawnCli({pipeStdToParent: false, logPrefix: "dev"}, [
// ⏎
"dev",
`--dataDir=${path.join(testFilesDir, "dev-voluntary-exit")}`,
Expand Down
9 changes: 7 additions & 2 deletions packages/cli/test/unit/util/loggerTransport.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import rimraf from "rimraf";
import {expect} from "chai";
import {config} from "@lodestar/config/default";
import {LodestarError, LogData, LogFormat, logFormats, LogLevel} from "@lodestar/utils";
import {getCliLogger, ILogArgs} from "../../../src/util/logger.js";
import {getCliLogger, ILogArgs, LOG_FILE_DISABLE_KEYWORD} from "../../../src/util/logger.js";

describe("winston logger format and options", () => {
interface ITestCase {
Expand Down Expand Up @@ -139,7 +139,12 @@ describe("winston transport log to file", () => {
});

function getCliLoggerTest(logArgs: Partial<ILogArgs>): ReturnType<typeof getCliLogger> {
return getCliLogger({dataDir: "", ...logArgs}, {defaultLogFile: ""}, config, {hideTimestamp: true});
return getCliLogger(
{logFile: LOG_FILE_DISABLE_KEYWORD, ...logArgs},
{defaultLogFilepath: "logger_transport_test.log"},
config,
{hideTimestamp: true}
);
}

/** Wait for file to exist have some content, then return its contents */
Expand Down
45 changes: 31 additions & 14 deletions packages/cli/test/utils/childprocRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,14 @@ const cliLibScriptPath = esmRelativePathJoin("../../lib/index.js");
/* eslint-disable no-console */

export type DescribeArgs = {
spawnCli(args: string[]): child_process.ChildProcessWithoutNullStreams;
spawnCli(opts: SpawnCliOpts, args: string[]): child_process.ChildProcessWithoutNullStreams;
};

type SpawnCliOpts = {
ensureProcRunning?: boolean;
logPrefix?: string;
pipeStdToParent?: boolean;
printOnlyOnError?: boolean;
};

// eslint-disable-next-line @typescript-eslint/explicit-function-return-type
Expand All @@ -35,8 +38,8 @@ export function describeCliTest(testName: string, callback: (this: Mocha.Suite,
});

const args: DescribeArgs = {
spawnCli(args: string[], opts?: SpawnCliOpts) {
const proc = spawnCli(args);
spawnCli(opts: SpawnCliOpts, args: string[]) {
const proc = spawnCli(opts, args);
console.log(`Created process ${proc.pid}`);

afterEachCallbacks.push(async function () {
Expand Down Expand Up @@ -70,9 +73,9 @@ export function describeCliTest(testName: string, callback: (this: Mocha.Suite,
});
}

export function spawnCli(lodestarArgs: string[]): child_process.ChildProcessWithoutNullStreams {
// TODO: Customize
const logPrefix = "vc";
export function spawnCli(opts: SpawnCliOpts, lodestarArgs: string[]): child_process.ChildProcessWithoutNullStreams {
let stdstr = "";
const logPrefix = opts?.logPrefix ?? "";

const command = RUN_FROM_SRC
? // ts-node --esm cli.ts
Expand All @@ -87,17 +90,31 @@ export function spawnCli(lodestarArgs: string[]): child_process.ChildProcessWith

const proc = child_process.spawn(command, prefixArgs);

proc.stdout.on("data", (chunk) => {
const str = Buffer.from(chunk).toString("utf8");
process.stdout.write(`${logPrefix} ${proc.pid}: ${str}`); // str already contains a new line. console.log adds a new line
});
proc.stderr.on("data", (chunk) => {
const str = Buffer.from(chunk).toString("utf8");
process.stderr.write(`${logPrefix} ${proc.pid}: ${str}`); // str already contains a new line. console.log adds a new line
});
if (opts?.pipeStdToParent) {
proc.stdout.on("data", (chunk) => {
const str = Buffer.from(chunk).toString("utf8");
process.stdout.write(`${logPrefix} ${proc.pid}: ${str}`); // str already contains a new line. console.log adds a new line
});
proc.stderr.on("data", (chunk) => {
const str = Buffer.from(chunk).toString("utf8");
process.stderr.write(`${logPrefix} ${proc.pid}: ${str}`); // str already contains a new line. console.log adds a new line
});
} else {
proc.stdout.on("data", (chunk) => {
stdstr += Buffer.from(chunk).toString("utf8");
});
proc.stderr.on("data", (chunk) => {
stdstr += Buffer.from(chunk).toString("utf8");
});
}

proc.on("exit", (code) => {
console.log("process exited", {code});
if (!opts?.pipeStdToParent) {
if (!opts?.printOnlyOnError || (code !== null && code > 0)) {
console.log(stdstr);
}
}
});

return proc;
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/test/utils/keymanagerTestRunners.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export function getKeymanagerTestRunner({args: {spawnCli}, afterEachCallbacks, d
afterEachCallbacks.push(() => beaconServer.close());
await beaconServer.listen();

const validatorProc = spawnCli([
const validatorProc = spawnCli({pipeStdToParent: true, logPrefix: "vc"}, [
// ⏎
"validator",
`--dataDir=${dataDir}`,
Expand Down