Skip to content

feat: add keyValueStore.getRecordPublicUrl #725

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

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!

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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants