Skip to content

fix: read local Actor input from the storage directory with non-filesystem storage clients - #1197

Open
Pijukatel wants to merge 3 commits into
masterfrom
sync/6fdf9e3d0164
Open

Pijukatel wants to merge 3 commits into
masterfrom
sync/6fdf9e3d0164

Conversation

@Pijukatel

Copy link
Copy Markdown
Contributor

Parity with apify/apify-sdk-js#811

Closes: #682

@Pijukatel
Pijukatel requested a review from szaganek as a code owner October 20, 2026 15:39
@github-actions github-actions Bot added the t-tooling Issues with this label are in the ownership of the tooling team. label Oct 20, 2026
@apify-service-account apify-service-account added the tested Temporary label used only programatically for some analytics. label Oct 20, 2026
@codecov

codecov Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.10%. Comparing base (b047e06) to head (eab0d7f).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1197      +/-   ##
==========================================
- Coverage   93.11%   93.10%   -0.02%     
==========================================
  Files          54       54              
  Lines        3691     3696       +5     
==========================================
+ Hits         3437     3441       +4     
- Misses        254      255       +1     
Flag Coverage Δ
e2e 31.60% <25.00%> (+0.01%) ⬆️
integration 51.59% <25.00%> (-0.02%) ⬇️
unit 89.88% <100.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@apify-factory apify-factory 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.

Looks good, one question about the lookup location.

🏭 Factory review — ℹ️ not auto-approved

This change was reviewed but cannot be auto-approved: a gate failed, so the review cannot end in an approval.

check result
Static gates ❌ 15/16 — file-extensions
Review findings ❌ not clean
Reviewer escalation ✅ none
Juror 1 — claude-sonnet-5-5, adversarial ⏭️ skipped — a gate failed
Why it was not auto-approved

Failed gates:

  • file-extensions: disallowed extension: docs/02_concepts/02_actor_input.mdx

The comments above are the review itself; this section only explains the approval decision. Push a change and request @apify-factory again to re-run it, or ask a human to review.

Reviewed 27b3e03de · workflow run

Comment thread src/apify/_actor.py Outdated
input_path = await self._find_input_file(input_key, Path.cwd(), 'working directory')
if input_path is None:
input_path = await self._find_input_file(
input_key, Path(self.configuration.storage_dir), 'storage directory'

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.

Question: issue #682 says the CLI writes its input into the default key-value store directory (<storage_dir>/key_value_stores/default/INPUT.json), but this only checks the root of storage_dir, so with a non-filesystem storage client the CLI-provided input is still ignored. Is the CLI meant to write <storage_dir>/INPUT.json instead?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Is the CLI meant to write <storage_dir>/INPUT.json instead?

Yes, that is my understanding.

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.

Actually, no, and this discrepancy is load-bearing. Let me fix the JS SDK before we merge this.

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.

@Pijukatel Pijukatel added the adhoc Ad-hoc unplanned task added during the sprint. label Oct 20, 2026

@apify-factory apify-factory 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.

Looks good, nothing blocking.

🏭 Factory review — ℹ️ not auto-approved

This change was reviewed but cannot be auto-approved: a gate failed, so the review cannot end in an approval.

check result
Static gates ❌ 15/16 — file-extensions
Review findings ✅ none
Reviewer escalation ✅ none
Juror 1 — claude-sonnet-5-5, adversarial ⏭️ skipped — a gate failed
Why it was not auto-approved

Failed gates:

  • file-extensions: disallowed extension: docs/02_concepts/02_actor_input.mdx

The comments above are the review itself; this section only explains the approval decision. Push a change and request @apify-factory again to re-run it, or ask a human to review.

Reviewed 27b3e03de · workflow run

Comment thread src/apify/_actor.py Outdated
Comment thread src/apify/_actor.py
input_value = await key_value_store.get_value(input_key)
if input_value is None and not self.is_at_home() and not await key_value_store.record_exists(input_key):
input_value = await self._read_input_from_working_directory(input_key)
input_path = await self._find_input_file(input_key, Path.cwd(), 'working directory')

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.

Note: This differs from JS (apify-sdk-js#811) in one edge case. JS chains the two lookups with ??, so a working-directory INPUT.json holding null falls through to the storage directory. Here the file that exists wins, even if it holds null. I think the Python behavior is the better one (and it's tested), so just flagging it in case parity matters.

✍️ Drafted by Claude Code

Comment thread src/apify/_actor.py Outdated
Co-authored-by: Vlada Dusek <v.dusek96@gmail.com>
@Pijukatel
Pijukatel requested a review from vdusek October 21, 2026 08:03

@vdusek vdusek 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.

LGTM

just - maybe this is more a fix than feat? WDYT?

(I updated it to "fix: read local Actor input from the storage directory with non-filesystem storage clients")

@vdusek vdusek changed the title feat: Actor.get_input looks inside the storage directory even on non-fs storage backends fix: read local Actor input from the storage directory with non-filesystem storage clients Oct 21, 2026

@janbuchar janbuchar 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.

@Pijukatel
Pijukatel requested a review from janbuchar October 21, 2026 14:46

This branch has not been deployed

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

Labels

adhoc Ad-hoc unplanned task added during the sprint. t-tooling Issues with this label are in the ownership of the tooling team. tested Temporary label used only programatically for some analytics.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Handle CLI with input for local development without FileSystemStorage client

5 participants