feat: add calculate-memory command to actor #980

danpoletaev · 2025-12-04T15:14:34Z

This PR adds new command to cli to allow developers to test their defaultMemoryMbytes expressions. Those expressions are used to dynamically calculate memory of the run based on the input and runOptions.

E.g. of usage

apify actor calculate-memory --input ./input.json --maxTotalChargeUsd=25 --timeoutSecs=360

Full specification in Notion 👇
https://www.notion.so/apify/Dynamic-default-Actor-memory-WIP-293f39950a2280b9aa34fa9cd07f52a4?source=copy_link

danpoletaev · 2025-12-04T15:25:31Z

test/local/commands/actor/calculate-memory.test.ts

+	writeFileSync(joinPath(LOCAL_CONFIG_PATH), JSON.stringify(actorJson, null, '\t'), { flag: 'w' });
+};
+
+describe('apify actor calculate-memory', () => {


@vladfrangu when I start it one by one it works perfectly, but when I start it as as a whole set it fails, can you help me to fix that, please? 🙃

It seems there's something wrong with path or existence of the file. I was a bit confused with that useTempPath mocks...

tobice

Nice 👏 Left some suggestions though.

tobice · 2025-12-05T06:26:18Z

src/commands/actor/calculate-memory.ts

+import { error, info, success } from '../../lib/outputs.js';
+import { getJsonFileContent } from '../../lib/utils.js';
+
+export class ActorCalculateMemoryCommand extends ApifyCommand<typeof ActorCalculateMemoryCommand> {


What is this magic? Why is it basically extending itself? 😅

It's used for types of args and flags, since they're different per command 😄

maybe @vladfrangu would add more info here

It's not extending itself but rather referencing itself in the extension class -> allows us to infer the types of the flags and args for this.args/this.flags

tobice · 2025-12-05T06:29:38Z

src/commands/actor/calculate-memory.ts

+	private readonly DEFAULT_INPUT_PATH = 'storage/key_value_stores/default/INPUT.json';
+
+	static override description =
+		`Calculates the actor’s dynamic memory usage based on a memory expression from actor.json, input data, and run options.`;


Suggested change

`Calculates the actor’s dynamic memory usage based on a memory expression from actor.json, input data, and run options.`;

`Calculates the Actor’s dynamic memory usage based on a memory expression from actor.json, input data, and run options.`;

tobice · 2025-12-05T06:45:47Z

src/commands/actor/calculate-memory.ts

+			description: 'Actor build version or build tag to evaluate the expression with.',
+			required: false,
+		}),
+		timeoutSecs: Flags.integer({


I can imagine this being slightly confusing (because it is confusing to me 😅 ).

Intuitively, why should restartOnError affect memory?

The only slightly related fields from run options are maxTotalChargeUsd or memoryMbytes but that latter one is not even here. And I forgot what it was supposed to do. Can you remind me? And is it intentional that it is not here?

The bottom line is that I find it really weird to have these top level params that have zero effect and are there simply because they are part of Actor run options.

I thought about dropping the options altogether from the memory calculation (that'd be a bigger change) but I guess maxTotalChargeUsd is useful. And we don't want to get into the business of deciding what is useful and what isn't.

I just wonder, is it possible to provide these also as a JSON file / object? Which would be probably inconsistent with other CLI commands but would make more intuitive sense.

Makes sense, I was also thinking about that as well. I'd not rather provide options as JSON, probably not many devs would need it.

The idea behind providing path to input.json is because new Actors already have default input and by providing defaultMemoryMbytes in actor.json devs can call apify actor calculate-memory without any flags.

Regarding memoryMbytes and diskMbytes I thought it would be very confusing and just removed them from flags, we can do the same for restartOnError.

IMHO it's cleaner for me to provide options as flags from DX point of view, but if you feel like it would be better to provide them in additional file I can do that as well.

The idea behind providing path to input.json is because new Actors already have default input and by providing defaultMemoryMbytes in actor.json devs can call apify actor calculate-memory without any flags.

Totally agree here 👍

Regarding memoryMbytes

Remind me, what is memoryMbytes doing?

IMHO it's cleaner for me to provide options as flags from DX point of view, but if you feel like it would be better to provide them in additional file I can do that as well.

From DX, I'd argue that providing a param that doesn't do anything is at least a little weird 😄

Remind me, what is memoryMbytes doing?

It's defaultRunOptions.memoryMbytes - fixed memory provided by the developer in settings.

From DX, I'd argue that providing a param that doesn't do anything is at least a little weird 😄

Makes sense, I removed restartOnError from flags and left a comment explaining this

tobice · 2025-12-05T09:55:21Z

src/commands/actor/calculate-memory.ts

+	/**
+	 * Helper to load the `defaultMemoryMbytes` expression from actor.json.
+	 */
+	private async getExpressionFromConfig(): Promise<string | undefined> {


(nit) There seems to be a lot of boilerplate and error handling for what seems to be a rather generic functionality: get Actor config 🤔 There is no existing utility?

Well we use one functionality useActorConfig, that already exists 😄 I'd wait for review from @vladfrangu here, I just couldn't find it

This is the existing utility, and it handles multiple cases (invalid config file, non-existing file or existing file).

I do think you could probably skip the exists == false check tho... It will just return undefined -> bubble up top

tobice · 2025-12-05T09:56:36Z

src/commands/actor/calculate-memory.ts

+		}
+
+		if (localConfigResult.isOkAnd((cfg) => cfg.exists === false)) {
+			throw new Error(


Why throwing here but using process.exitCode above? 🤔

I assume it's a CLI thing but I don't follow the reasoning 😄

Good point, again let's wait for best practices from tooling team. I was not sure about this one as well 🙃

we set process.exitCode in certain cases for those who use shells and $? to differentiate exit reasons...

RE: why throw -> I honestly want to move away from throws and more towards handling errors in the cmd level (building one generic catch handler for all errors is rough, unless we build custom error classes [but at that point just implement the nicer error msgs where they're thrown])

tobice · 2025-12-05T09:57:43Z

test/local/commands/actor/calculate-memory.test.ts

+		expect(lastErrorMessage()).toMatch(/Calculated memory: 512 MB/);
+	});
+
+	it('should calculate memory using expression from .actor.json', async () => {


Suggested change

it('should calculate memory using expression from .actor.json', async () => {

it('should calculate memory using expression from actor.json', async () => {

tobice · 2025-12-05T10:01:11Z

test/local/commands/actor/calculate-memory.test.ts

+	it('should calculate memory using defaultMemoryMbytes flag', async () => {
+		await testRunCommand(ActorCalculateMemoryCommand, {
+			flags_input: 'INPUT.json',
+			flags_defaultMemoryMbytes: '512',


It's confusing that when providing the expression via a flag, you use just a constant value, but when providing from actor.json, you use an actual expression. That creates the impression that it works differently but as far as I can tell, it doesn't?

I'd just use the same thing in both (actually all, dtto below) cases.

What you can do is something like:

const START_URLS_LENGTH_BASED_MEMORY_EXPRESSION = 'get(input, 'startUrls.length') * 1024';

And then reuse it across all cases. That will make it super clear that the expression itself doesn't really matter and remains constant across cases.

tobice · 2025-12-05T10:05:30Z

@danpoletaev Can you also include another person from our team for the sake of our bus factor on the feature? 🙏

vladfrangu

I can help out with the tests if they're still causing you lots of issues 🫡, just lmk!

vladfrangu · 2025-12-10T07:46:33Z

src/commands/actor/calculate-memory.ts

+export class ActorCalculateMemoryCommand extends ApifyCommand<typeof ActorCalculateMemoryCommand> {
+	static override name = 'calculate-memory' as const;
+
+	private readonly DEFAULT_INPUT_PATH = 'storage/key_value_stores/default/INPUT.json';


Please use path.join + getLocalKeyValueStorePath

Also this can be moved to a constant outside the class

vladfrangu · 2025-12-10T07:48:44Z

src/commands/actor/calculate-memory.ts

+		input: Flags.string({
+			description: 'Path to the input JSON file used for the calculation.',
+			required: false,
+			default: 'storage/key_value_stores/default/INPUT.json',


Either reference the path via this. (/ const var name if moved), or don't provide a default at all and set it in code when destructuring the flag (const { input = DEFAULT_INPUT_PATH } = this.flags)

vladfrangu · 2025-12-10T07:49:43Z

src/commands/actor/calculate-memory.ts

+			required: false,
+		}),
+		restartOnError: Flags.boolean({
+			description: 'Whether to restart the actor on error.',


Suggested change

description: 'Whether to restart the actor on error.',

description: 'Whether the Actor will be restarted on error.',

vladfrangu · 2025-12-10T07:53:18Z

src/commands/actor/calculate-memory.ts

+	/**
+	 * Helper to load the `defaultMemoryMbytes` expression from actor.json.
+	 */
+	private async getExpressionFromConfig(): Promise<string | undefined> {


This is the existing utility, and it handles multiple cases (invalid config file, non-existing file or existing file).

I do think you could probably skip the exists == false check tho... It will just return undefined -> bubble up top

vladfrangu · 2025-12-10T07:55:13Z

src/commands/actor/calculate-memory.ts

+		}
+
+		if (localConfigResult.isOkAnd((cfg) => cfg.exists === false)) {
+			throw new Error(


we set process.exitCode in certain cases for those who use shells and $? to differentiate exit reasons...

RE: why throw -> I honestly want to move away from throws and more towards handling errors in the cmd level (building one generic catch handler for all errors is rough, unless we build custom error classes [but at that point just implement the nicer error msgs where they're thrown])

vladfrangu · 2025-12-10T07:57:42Z

test/local/commands/actor/calculate-memory.test.ts

+	beforeEach(async () => {
+		await beforeAllCalls();
+		toggleCwdBetweenFullAndParentPath();
+	});
+
+	afterEach(async () => {
+		await afterAllCalls();
+	});
+


I don't think you should call toggle every time, right?

Good point 👍

vladfrangu · 2025-12-10T07:58:14Z

src/commands/actor/calculate-memory.ts

+				input: inputJson,
+				runOptions,
+			});
+			success({ message: `Calculated memory: ${result} MB` });


provide the stdout: true option (and adjust the test accordingly) 🙏

vladfrangu · 2025-12-10T07:58:49Z

test/local/commands/actor/calculate-memory.test.ts

+	});
+
+	it('should fallback to default input path if input flag is not provided', async () => {
+		const defaultInputPath = joinPath('storage/key_value_stores/default');


use multiple args passed to this function, not a full path, for OS compat

vladfrangu · 2025-12-10T07:59:05Z

test/local/commands/actor/calculate-memory.test.ts

+	it('should fail when default memory is not provided and actor.json is missing', async () => {
+		await testRunCommand(ActorCalculateMemoryCommand, {});
+
+		expect(lastErrorMessage()).toMatch(/actor.json not found at/);


Suggested change

expect(lastErrorMessage()).toMatch(/actor.json not found at/);

expect(lastErrorMessage()).toMatch(/actor\.json not found at/);

vladfrangu · 2025-12-10T07:59:25Z

test/local/commands/actor/calculate-memory.test.ts

+		});
+
+		// INPUT.json does not exist, so input.startUrls.length will be undefined, defaulting to 1
+		expect(lastErrorMessage()).toMatch(/Calculated memory: 1024 MB/);


I would assert only the memory requested, not the whole msg

tobice

Thanks!

tobice · 2025-12-12T13:37:32Z

docs/reference.md

 ```sh
 DESCRIPTION
-  Creates an Actor project from a template in a new directory.
+  Creates an Actor project from a template in a new directory. The command 


Why these changes? They seem unrelated.

This was caused by automatic doc generation, so I think it is fine

feat: add calculate-memory command to actor

5d682c8

danpoletaev self-assigned this Dec 4, 2025

danpoletaev requested a review from vladfrangu as a code owner December 4, 2025 15:14

github-actions bot added this to the 129th sprint - Core Services team milestone Dec 4, 2025

github-actions bot added t-core-services Issues with this label are in the ownership of the core services team. tested Temporary label used only programatically for some analytics. labels Dec 4, 2025

refactor: clean up

d740ebe

danpoletaev commented Dec 4, 2025

View reviewed changes

danpoletaev requested a review from tobice December 4, 2025 15:26

tobice requested changes Dec 5, 2025

View reviewed changes

refactor: clean up

ce6f5cd

danpoletaev requested a review from nmanerikar December 5, 2025 13:05

vladfrangu requested changes Dec 10, 2025

View reviewed changes

danpoletaev added 3 commits December 11, 2025 10:10

refactor: clean up

fa505f8

refactor: clean up

0bd55a8

fix: docs build

fc6c5fd

danpoletaev requested a review from TC-MO as a code owner December 11, 2025 09:40

danpoletaev added 3 commits December 11, 2025 10:50

refactor: clean up

a5c5662

fix: allow absolute path

5ad13ac

refactor: clean up

4cab2e6

danpoletaev requested review from tobice and vladfrangu December 11, 2025 10:03

tobice approved these changes Dec 12, 2025

View reviewed changes

vladfrangu approved these changes Dec 16, 2025

View reviewed changes

Merge branch 'master' into feat/calculate-run-dynamic-memory

fcba54b

danpoletaev merged commit 78eeee7 into master Dec 16, 2025
21 of 22 checks passed

danpoletaev deleted the feat/calculate-run-dynamic-memory branch December 16, 2025 16:05

	`Calculates the actor’s dynamic memory usage based on a memory expression from actor.json, input data, and run options.`;
	`Calculates the Actor’s dynamic memory usage based on a memory expression from actor.json, input data, and run options.`;

	it('should calculate memory using expression from .actor.json', async () => {
	it('should calculate memory using expression from actor.json', async () => {

	description: 'Whether to restart the actor on error.',
	description: 'Whether the Actor will be restarted on error.',

	expect(lastErrorMessage()).toMatch(/actor.json not found at/);
	expect(lastErrorMessage()).toMatch(/actor\.json not found at/);

feat: add calculate-memory command to actor #980

feat: add calculate-memory command to actor #980

Uh oh!

Conversation

danpoletaev commented Dec 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tobice left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danpoletaev Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danpoletaev Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tobice commented Dec 5, 2025

Uh oh!

vladfrangu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tobice left a comment

Choose a reason for hiding this comment

Uh oh!

danpoletaev commented Dec 4, 2025 •

edited

Loading

danpoletaev Dec 5, 2025 •

edited

Loading

danpoletaev Dec 11, 2025 •

edited

Loading