Skip to content
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

feat: add sorted-set examples #241

Merged
merged 2 commits into from
Oct 25, 2024
Merged

feat: add sorted-set examples #241

merged 2 commits into from
Oct 25, 2024

Conversation

rishtigupta
Copy link
Contributor

PR Description:

  • Update the composer.json to reference the main branch of client-sdk-php repo since we currently do not have a released sdk version that contains the sorted-set apis
  • Add sorted-set examples

Issue

#213
https://github.com/momentohq/dev-eco-issue-tracker/issues/1033

Copy link
Collaborator

@malandis malandis left a comment

Choose a reason for hiding this comment

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

Lgtm!

@@ -1,6 +1,7 @@
{
"require": {
"momentohq/client-sdk-php": "1.11.1",
"momentohq/client-sdk-php": "dev-main",
Copy link
Collaborator

Choose a reason for hiding this comment

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

🎗️ to revert this change once the new SDK is released

@rishtigupta rishtigupta force-pushed the feat/missing-examples branch from 24068ad to 0ad6e75 Compare October 25, 2024 17:50
@rishtigupta rishtigupta requested a review from malandis October 25, 2024 17:50

// fetch sorted set by rank (0 to -1 for all elements) in ascending order
$logger->info("Fetching sorted set $SET_NAME\n");
$response = $client->sortedSetFetchByRank($CACHE_NAME, $SET_NAME, 0, -1);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not leave off the ranks for fetching all elements?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am looking into that. It was giving me error saying:

[ex:]: Error fetching sorted set: Invalid argument passed to Momento client: minScore must be less than or equal to maxScore

Copy link
Contributor

@nand4011 nand4011 left a comment

Choose a reason for hiding this comment

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

I think we should show fetching in descending order using a named argument if possible, LGTM overall. We should get this in.

@rishtigupta rishtigupta merged commit 1906221 into main Oct 25, 2024
5 checks passed
@rishtigupta rishtigupta deleted the feat/missing-examples branch October 25, 2024 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants