Repository navigation
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 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
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
apify-factory
left a comment
There was a problem hiding this comment.
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
| 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' |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Is the CLI meant to write <storage_dir>/INPUT.json instead?
Yes, that is my understanding.
There was a problem hiding this comment.
Actually, no, and this discrepancy is load-bearing. Let me fix the JS SDK before we merge this.
apify-factory
left a comment
There was a problem hiding this comment.
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
| 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') |
There was a problem hiding this comment.
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
Co-authored-by: Vlada Dusek <v.dusek96@gmail.com>
Actor.get_input looks inside the storage directory even on non-fs storage backends…stead of the storage root
Parity with apify/apify-sdk-js#811
Closes: #682