Skip to content

feat: add keyValueStore.getRecordPublicUrl#725

Merged
danpoletaev merged 3 commits intomasterfrom
feat/get-record-public-url
Aug 11, 2025
Merged

feat: add keyValueStore.getRecordPublicUrl#725
danpoletaev merged 3 commits intomasterfrom
feat/get-record-public-url

Conversation

@danpoletaev
Copy link
Contributor

This PR adds new method to KV store client:

  • getRecordPublicUrl(key: string)

Note: We already have this same method (KeyValueStore.getPublicUrl) in SDK.

@danpoletaev danpoletaev requested review from janbuchar and tobice August 6, 2025 13:26
@danpoletaev danpoletaev self-assigned this Aug 6, 2025
@danpoletaev danpoletaev added the adhoc Ad-hoc unplanned task added during the sprint. label Aug 6, 2025
@github-actions 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 Aug 6, 2025
@janbuchar
Copy link
Contributor

Pardon me being blunt, but what's the endgame here? Are users expected to stop using KeyValueStore.getPublicUrl and use this instead? Should both co-exist in harmony?

@danpoletaev
Copy link
Contributor Author

Pardon me being blunt, but what's the endgame here? Are users expected to stop using KeyValueStore.getPublicUrl and use this instead? Should both co-exist in harmony?

KeyValueStore.getPublicUrl is SDK method, now I've added same logic to the client. While creating new Actors, people would use mainly SDK, but there would be other cases when you'd want to use it without SDK 🙃

@janbuchar
Copy link
Contributor

Pardon me being blunt, but what's the endgame here? Are users expected to stop using KeyValueStore.getPublicUrl and use this instead? Should both co-exist in harmony?

KeyValueStore.getPublicUrl is SDK method, now I've added same logic to the client. While creating new Actors, people would use mainly SDK, but there would be other cases when you'd want to use it without SDK 🙃

Huh, okay, what would those cases be? From the perspective of an Actor developer (or anyone primarily using the SDK), having two slightly different (or are they?) ways to achieve the same thing is very confusing.

@danpoletaev
Copy link
Contributor Author

Pardon me being blunt, but what's the endgame here? Are users expected to stop using KeyValueStore.getPublicUrl and use this instead? Should both co-exist in harmony?

KeyValueStore.getPublicUrl is SDK method, now I've added same logic to the client. While creating new Actors, people would use mainly SDK, but there would be other cases when you'd want to use it without SDK 🙃

Huh, okay, what would those cases be? From the perspective of an Actor developer (or anyone primarily using the SDK), having two slightly different (or are they?) ways to achieve the same thing is very confusing.

For example you just want to use our API, why would you download whole Apify SDK with Crawlee for that?
I suppose our integration team will mostly use client instead of SDK.

Also, in apify-cli I need this method to sign a tarball ZIP of the build.

Methods are nearly identical in SDK and client

@janbuchar
Copy link
Contributor

Pardon me being blunt, but what's the endgame here? Are users expected to stop using KeyValueStore.getPublicUrl and use this instead? Should both co-exist in harmony?

KeyValueStore.getPublicUrl is SDK method, now I've added same logic to the client. While creating new Actors, people would use mainly SDK, but there would be other cases when you'd want to use it without SDK 🙃

Huh, okay, what would those cases be? From the perspective of an Actor developer (or anyone primarily using the SDK), having two slightly different (or are they?) ways to achieve the same thing is very confusing.

For example you just want to use our API, why would you download whole Apify SDK with Crawlee for that? I suppose our integration team will mostly use client instead of SDK.

Also, in apify-cli I need this method to sign a tarball ZIP of the build.

Yeah, I get that sometimes you want to use the client without pulling in the SDK along with crawlee as a dependency.

Methods are nearly identical in SDK and client

And that's probably my main issue. If we make the KeyValueStore.getPublicUrl async in crawlee v4, I guess we should be able to make it simply delegate to the client?

Copy link
Contributor

@tobice tobice left a comment

Choose a reason for hiding this comment

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

Approving. Agreed with the points here... the end game should be that we can just proxy the client implementation from SDK.

And perhaps the naming should be unified 😄

* Generates a URL that can be used to access key-value store record.
*
* If the client has permission to access the key-value store's URL signing key,
* the URL will include a signature to verify its authenticity.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* the URL will include a signature to verify its authenticity.
* the URL will include a signature which will allow the link to work even without authentication.

@janbuchar
Copy link
Contributor

And perhaps the naming should be unified 😄

Yes please!

@danpoletaev
Copy link
Contributor Author

@janbuchar agreed with Tobi, that we'll call those new client methods in SDK(so we'll not copy same functionality over and over).
We agreed to wait a bit, before we'll add those methods to SDK clients, since it is not priority as of now.

Regarding naming, we'll need to deprecate later KeyValueStore.getPublicUrl in SDK and replace it with async KeyValueStore.getRecordPublicUrl, which will just call client

@danpoletaev danpoletaev merged commit d84a03a into master Aug 11, 2025
7 checks passed
@danpoletaev danpoletaev deleted the feat/get-record-public-url branch August 11, 2025 08:39
@tobice tobice added the validated Issues that are resolved and their solutions fulfill the acceptance criteria. label Aug 11, 2025
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-core-services Issues with this label are in the ownership of the core services team. tested Temporary label used only programatically for some analytics. validated Issues that are resolved and their solutions fulfill the acceptance criteria.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants