Skip to content

feat: read applyEnvVarsToBuild from actor.json in push (#734) - #1346

Open
MatousMarik wants to merge 4 commits into
feat/push-apply-env-vars-to-build-734from
feat/actor-json-apply-env-vars-to-build-734
Open

feat: read applyEnvVarsToBuild from actor.json in push (#734)#1346
MatousMarik wants to merge 4 commits into
feat/push-apply-env-vars-to-build-734from
feat/actor-json-apply-env-vars-to-build-734

Conversation

@MatousMarik

@MatousMarik MatousMarik commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Part of #734, stacked on #1345 (retarget to master once that merges — only the last commit is new here).

What

  • apify push now reads applyEnvVarsToBuild from .actor/actor.json, so the setting persists per repo and applies on every push (useful for CI and Actors needing build-time secrets).
  • Precedence: --[no-]apply-env-vars-to-build flag > applyEnvVarsToBuild in actor.json > omitted (value stored on the platform is kept). An explicit false in actor.json turns the setting off.
  • Documented in docs/vars.md; flag description updated (+ regenerated docs/reference.md).

Tests

  • New [api] test covering field true, field false, flag-over-field precedence, and the negated flag beating a true in the file (the case that pins ?? over ||). Full push API suite passes (14/14), plus test:local, lint, format, build.
  • Note for reviewers: useActorConfig caches by cwd, so the test resets the cache when rewriting actor.json mid-test.

Follow-up (separate PR)

No new dependencies; no install-size impact.

@MatousMarik
MatousMarik force-pushed the feat/push-apply-env-vars-to-build-734 branch from 9c96569 to 923d4a8 Compare August 25, 2026 07:42
@MatousMarik
MatousMarik force-pushed the feat/actor-json-apply-env-vars-to-build-734 branch from 556d4c0 to c53d79f Compare August 25, 2026 07:42
@MatousMarik
MatousMarik force-pushed the feat/actor-json-apply-env-vars-to-build-734 branch from c53d79f to 9cfb129 Compare August 25, 2026 08:08
The field persists the build env vars setting per repo, so it applies
on every push without remembering the flag. The
--apply-env-vars-to-build flag takes precedence over the field, and an
explicit false in actor.json turns the setting off. When both are
omitted, the value stored on the platform is kept.
@MatousMarik
MatousMarik force-pushed the feat/push-apply-env-vars-to-build-734 branch from 923d4a8 to 420860a Compare August 25, 2026 09:16
@MatousMarik
MatousMarik force-pushed the feat/actor-json-apply-env-vars-to-build-734 branch from 9cfb129 to a8a1158 Compare August 25, 2026 09:16
@MatousMarik
MatousMarik marked this pull request as ready for review August 25, 2026 13:23
@l2ysho
l2ysho requested a review from apify-factory August 26, 2026 08:40

@apify-factory apify-factory left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks good, nothing blocking.

@szaganek szaganek left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Three suggestions from me to improve readability.

Comment thread docs/vars.md Outdated
Comment thread docs/vars.md Outdated
Comment thread src/commands/actors/push.ts Outdated
'apply-env-vars-to-build': Flags.boolean({
description:
'Make the environment variables also available to the Actor build process. Use --no-apply-env-vars-to-build to turn the setting off. When omitted, the setting currently stored on the platform is kept.',
description: `Make the environment variables also available to the Actor build process. Use --no-apply-env-vars-to-build to turn the setting off. Overrides the 'applyEnvVarsToBuild' field in the '${LOCAL_CONFIG_PATH}' file. When both are omitted, the setting currently stored on the platform is kept.`,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
description: `Make the environment variables also available to the Actor build process. Use --no-apply-env-vars-to-build to turn the setting off. Overrides the 'applyEnvVarsToBuild' field in the '${LOCAL_CONFIG_PATH}' file. When both are omitted, the setting currently stored on the platform is kept.`,
description: `Make the environment variables also available to the Actor build process. To turn the setting off, use --no-apply-env-vars-to-build. Overrides the value of the 'applyEnvVarsToBuild' field in the '${LOCAL_CONFIG_PATH}' file. Without a flag, the setting currently stored on the platform is kept.`,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[claude] -> original version seems to be better, platform is kept when both are omitted, not just flag.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

feel free to resolve as you like cc @szaganek

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah. Let's make it explicit then:
When both the field and the flag are omitted, the setting currently stored on the platform is kept.

MatousMarik and others added 3 commits August 27, 2026 15:44
Co-authored-by: Edyta <142720610+szaganek@users.noreply.github.com>
Co-authored-by: Edyta <142720610+szaganek@users.noreply.github.com>
Co-authored-by: Edyta <142720610+szaganek@users.noreply.github.com>

@l2ysho l2ysho left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

thx @MatousMarik
pre-aprooving as nothing blocking here but I think pnpm run update-docs should be made, also I left one comment in docs thread where It looks like original version was more inline with code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants