Skip to content

refactor!: Rename StorageManager to StorageInstanceManager and use it to centralize storage instance caching#3584

Merged
janbuchar merged 22 commits into
v4from
refactor-storage-manager
May 11, 2026
Merged

refactor!: Rename StorageManager to StorageInstanceManager and use it to centralize storage instance caching#3584
janbuchar merged 22 commits into
v4from
refactor-storage-manager

Conversation

@janbuchar

@janbuchar janbuchar commented Apr 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

@janbuchar janbuchar added the t-tooling Issues with this label are in the ownership of the tooling team. label Apr 20, 2026
@janbuchar
janbuchar requested review from B4nan, barjin and l2ysho April 20, 2026 16:39
@janbuchar
janbuchar changed the base branch from master to storages-refactor April 20, 2026 16:39
@janbuchar janbuchar changed the title refactor\!: Make the StorageManager more like StorageInstanceManager from crawlee-python refactor!: Make the StorageManager more like StorageInstanceManager from crawlee-python Apr 20, 2026
@janbuchar janbuchar self-assigned this Apr 21, 2026
Base automatically changed from storages-refactor to v4 April 22, 2026 12:45
@janbuchar
janbuchar force-pushed the refactor-storage-manager branch from fc1d565 to afc367c Compare April 22, 2026 13:59
@janbuchar
janbuchar force-pushed the refactor-storage-manager branch from fc3505c to 4f69d63 Compare April 23, 2026 18:54
janbuchar
janbuchar commented Apr 23, 2026
View reviewed changes
Comment thread packages/core/src/storages/storage_instance_manager.ts
@janbuchar
janbuchar marked this pull request as ready for review April 23, 2026 20:01
barjin
barjin reviewed Apr 24, 2026
View reviewed changes

@barjin barjin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you @janbuchar !

Here are a few ideas while I still have the context from the previous PR.

Comment thread test/core/crawlers/basic_crawler.test.ts
Comment thread packages/types/src/storages.ts Outdated
Comment thread packages/core/src/storages/storage_instance_manager.ts Outdated
Comment on lines +231 to +234
client,
config,
clientOpener,
clientCacheKey,

@barjin barjin Apr 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

With client, clientOpener, and clientCacheKey, the client- prefix is imo too overloaded to read this comfortably.

Maybe it's time to switch to the backend naming? iiuc, client and clientCacheKey refer to the "big" client (backend), and clientOpener refers to the DatasetClient etc., right?

Comment thread packages/core/src/storages/storage_instance_manager.ts Outdated
Comment thread packages/core/src/storages/storage_instance_manager.ts
Comment thread packages/core/src/storages/storage_instance_manager.ts Outdated
@janbuchar
janbuchar requested a review from barjin May 4, 2026 16:05
barjin
barjin reviewed May 5, 2026
View reviewed changes

@barjin barjin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you @janbuchar , I have a few more ideas / questions, see below ⬇️

Comment thread packages/basic-crawler/src/internals/basic-crawler.ts Outdated
// pass in failed requests back to the `crawler.run()`, otherwise they would be considered as handled and
// ignored - as a failed requests is still handled.
if (this.requestQueue?.name === 'default' && purgeRequestQueue) {
const isDefaultQueue = this.requestQueue?.name === 'default' || this.requestQueue?.name === '__default__';

@barjin barjin May 5, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
const isDefaultQueue = this.requestQueue?.name === 'default' || this.requestQueue?.name === '__default__';
const isDefaultQueue = this.requestQueue?.name === 'default' || this.requestQueue?.alias === '__default__';

Is this right? I haven't got used to this new paradigm yet, but __default__ seems to be referenced to as an alias throughout this PR.

@janbuchar janbuchar May 7, 2026

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.

No, that actually worked only for memory storage and we'll completely rewrite that one soon. I reverted the change here and instead made a workaround in memory storage - 3cda48c

Comment thread packages/core/src/service_locator.ts Outdated
Comment thread packages/core/src/storages/storage_instance_manager.ts Outdated
@janbuchar

This comment was marked as resolved.

Sign in to view
@janbuchar
janbuchar requested a review from barjin May 7, 2026 12:00
@janbuchar janbuchar mentioned this pull request May 7, 2026
Closed
@janbuchar janbuchar linked an issue May 7, 2026 that may be closed by this pull request
Refactor StorageManager to match the Python counterpart #3592
Closed
@janbuchar janbuchar changed the title refactor!: Make the StorageManager more like StorageInstanceManager from crawlee-python refactor!: Rename StorageManager to StorageInstanceManager and use it to centralize storage instance caching May 11, 2026
@janbuchar
janbuchar merged commit 20b320a into v4 May 11, 2026
6 checks passed
@janbuchar
janbuchar deleted the refactor-storage-manager branch May 11, 2026 11:18
B4nan added a commit to apify/apify-sdk-js that referenced this pull request May 12, 2026
@B4nan @claude
fix(storage): adapt to crawlee v4 beta.56 storage rename
112d8f2 
beta.56 (apify/crawlee#3584) renamed `StorageManager` →
`StorageInstanceManager` and reshaped the public storage open path.
The static `StorageManager.openStorage(cls, id, client)` helper is
gone; each storage class now exposes `static open(id, options?)` with
a `storageClient` option for routing through a custom backend.

- `actor.ts`: `_openStorage` now calls `storageClass.open(id, { storageClient })`
  instead of `StorageManager.openStorage(...)`. `StorageOpenOptions` replaces
  `StorageManagerOptions`.
- `key_value_store.ts`: import `StorageOpenOptions` for the `open()` override
  signature.
- `actor.test.ts`: the `openDataset` / `openRequestQueue` `forceCloud` tests
  now spy on the storage class's own `open()` (no more `StorageManager.prototype`),
  and assert the `storageClient` lives one level deeper in the options object.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
B4nan added a commit to apify/apify-sdk-js that referenced this pull request May 12, 2026
@B4nan @claude
fix(storage): adapt to crawlee v4 beta.56 storage rename
5ea0f5e 
beta.56 (apify/crawlee#3584) renamed `StorageManager` →
`StorageInstanceManager` and reshaped the public storage open path.
The static `StorageManager.openStorage(cls, id, client)` helper is
gone; each storage class now exposes `static open(id, options?)` with
a `storageClient` option for routing through a custom backend.

- `actor.ts`: `_openStorage` now calls `storageClass.open(id, { storageClient })`
  instead of `StorageManager.openStorage(...)`. `StorageOpenOptions` replaces
  `StorageManagerOptions`.
- `key_value_store.ts`: import `StorageOpenOptions` for the `open()` override
  signature.
- `actor.test.ts`: the `openDataset` / `openRequestQueue` `forceCloud` tests
  now spy on the storage class's own `open()` (no more `StorageManager.prototype`),
  and assert the `storageClient` lives one level deeper in the options object.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
B4nan added a commit to apify/apify-sdk-js that referenced this pull request May 13, 2026
@B4nan @claude
fix(storage): adapt to crawlee v4 beta.56 storage rename
02eaf50 
beta.56 (apify/crawlee#3584) renamed `StorageManager` →
`StorageInstanceManager` and reshaped the public storage open path.
The static `StorageManager.openStorage(cls, id, client)` helper is
gone; each storage class now exposes `static open(id, options?)` with
a `storageClient` option for routing through a custom backend.

- `actor.ts`: `_openStorage` now calls `storageClass.open(id, { storageClient })`
  instead of `StorageManager.openStorage(...)`. `StorageOpenOptions` replaces
  `StorageManagerOptions`.
- `key_value_store.ts`: import `StorageOpenOptions` for the `open()` override
  signature.
- `actor.test.ts`: the `openDataset` / `openRequestQueue` `forceCloud` tests
  now spy on the storage class's own `open()` (no more `StorageManager.prototype`),
  and assert the `storageClient` lives one level deeper in the options object.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
B4nan added a commit to apify/apify-sdk-js that referenced this pull request Jun 2, 2026
@B4nan @claude
fix(storage): adapt to crawlee v4 beta.56 storage rename
d97d9c1 
beta.56 (apify/crawlee#3584) renamed `StorageManager` →
`StorageInstanceManager` and reshaped the public storage open path.
The static `StorageManager.openStorage(cls, id, client)` helper is
gone; each storage class now exposes `static open(id, options?)` with
a `storageClient` option for routing through a custom backend.

- `actor.ts`: `_openStorage` now calls `storageClass.open(id, { storageClient })`
  instead of `StorageManager.openStorage(...)`. `StorageOpenOptions` replaces
  `StorageManagerOptions`.
- `key_value_store.ts`: import `StorageOpenOptions` for the `open()` override
  signature.
- `actor.test.ts`: the `openDataset` / `openRequestQueue` `forceCloud` tests
  now spy on the storage class's own `open()` (no more `StorageManager.prototype`),
  and assert the `storageClient` lives one level deeper in the options object.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
barjin pushed a commit that referenced this pull request Jul 20, 2026
@janbuchar @barjin
refactor!: Rename StorageManager to StorageInstanceManager and use it…
4aed3bf 
… to centralize storage instance caching (#3584)

- depends on #3570
- closes #3592
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@barjin barjin barjin approved these changes
@B4nan B4nan Awaiting requested review from B4nan
@l2ysho l2ysho Awaiting requested review from l2ysho

Assignees

@janbuchar janbuchar

Labels

t-tooling Issues with this label are in the ownership of the tooling team.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Refactor StorageManager to match the Python counterpart

4 participants

@janbuchar @B4nan @barjin @apify-service-account