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

Support JSON.MSET Command #131

Merged
merged 9 commits into from
May 10, 2023
Merged

Support JSON.MSET Command #131

merged 9 commits into from
May 10, 2023

Conversation

shacharPash
Copy link
Collaborator

Closes #128

@shacharPash shacharPash temporarily deployed to REDIS_USER May 8, 2023 11:59 — with GitHub Actions Inactive
@shacharPash shacharPash temporarily deployed to REDIS_USER May 8, 2023 11:59 — with GitHub Actions Inactive
@shacharPash shacharPash temporarily deployed to REDIS_USER May 8, 2023 11:59 — with GitHub Actions Inactive
@shacharPash shacharPash temporarily deployed to REDIS_USER May 8, 2023 11:59 — with GitHub Actions Inactive
@shacharPash
Copy link
Collaborator Author

@chayim @oshadmi
The changes of RedisJson need to be pushed to redis-stack-server:edge so that it is possible to run tests on this command in CI

@shacharPash shacharPash temporarily deployed to REDIS_USER May 8, 2023 13:50 — with GitHub Actions Inactive
@shacharPash shacharPash temporarily deployed to REDIS_USER May 8, 2023 13:50 — with GitHub Actions Inactive
@shacharPash shacharPash temporarily deployed to REDIS_USER May 8, 2023 13:50 — with GitHub Actions Inactive
@shacharPash shacharPash temporarily deployed to REDIS_USER May 8, 2023 13:50 — with GitHub Actions Inactive
@shacharPash shacharPash temporarily deployed to REDIS_USER May 8, 2023 13:55 — with GitHub Actions Inactive
@shacharPash shacharPash temporarily deployed to REDIS_USER May 8, 2023 13:55 — with GitHub Actions Inactive
@shacharPash shacharPash temporarily deployed to REDIS_USER May 8, 2023 13:55 — with GitHub Actions Inactive
@shacharPash shacharPash temporarily deployed to REDIS_USER May 8, 2023 13:55 — with GitHub Actions Inactive
@shacharPash shacharPash temporarily deployed to REDIS_USER May 8, 2023 14:05 — with GitHub Actions Inactive
@shacharPash shacharPash temporarily deployed to REDIS_USER May 8, 2023 14:05 — with GitHub Actions Inactive
@shacharPash shacharPash temporarily deployed to REDIS_USER May 8, 2023 14:05 — with GitHub Actions Inactive
@shacharPash shacharPash temporarily deployed to REDIS_USER May 8, 2023 14:05 — with GitHub Actions Inactive
@codecov-commenter
Copy link

codecov-commenter commented May 8, 2023

Codecov Report

Patch coverage: 10.34% and project coverage change: -0.53 ⚠️

Comparison is base (09dfce7) 93.25% compared to head (eabd24d) 92.73%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #131      +/-   ##
==========================================
- Coverage   93.25%   92.73%   -0.53%     
==========================================
  Files          77       78       +1     
  Lines        4582     4611      +29     
  Branches      424      427       +3     
==========================================
+ Hits         4273     4276       +3     
- Misses        187      212      +25     
- Partials      122      123       +1     
Impacted Files Coverage Δ
src/NRedisStack/Json/JsonCommandBuilder.cs 87.89% <0.00%> (-3.50%) ⬇️
src/NRedisStack/Json/JsonCommands.cs 85.50% <0.00%> (-1.91%) ⬇️
src/NRedisStack/Json/JsonCommandsAsync.cs 85.50% <0.00%> (-1.91%) ⬇️
src/NRedisStack/Json/DataTypes/KeyValuePath.cs 17.64% <17.64%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@slorello89 slorello89 temporarily deployed to REDIS_USER May 9, 2023 20:41 — with GitHub Actions Inactive
@slorello89 slorello89 temporarily deployed to REDIS_USER May 9, 2023 20:41 — with GitHub Actions Inactive
@slorello89 slorello89 temporarily deployed to REDIS_USER May 9, 2023 20:41 — with GitHub Actions Inactive
@slorello89 slorello89 temporarily deployed to REDIS_USER May 9, 2023 20:41 — with GitHub Actions Inactive
@slorello89 slorello89 temporarily deployed to REDIS_USER May 9, 2023 21:00 — with GitHub Actions Inactive
@slorello89 slorello89 temporarily deployed to REDIS_USER May 9, 2023 21:00 — with GitHub Actions Inactive
@slorello89 slorello89 temporarily deployed to REDIS_USER May 9, 2023 21:00 — with GitHub Actions Inactive
@slorello89 slorello89 temporarily deployed to REDIS_USER May 9, 2023 21:00 — with GitHub Actions Inactive
@slorello89 slorello89 temporarily deployed to REDIS_USER May 9, 2023 21:09 — with GitHub Actions Inactive
@slorello89 slorello89 temporarily deployed to REDIS_USER May 9, 2023 21:09 — with GitHub Actions Inactive
@slorello89 slorello89 temporarily deployed to REDIS_USER May 9, 2023 21:09 — with GitHub Actions Inactive
@slorello89 slorello89 temporarily deployed to REDIS_USER May 9, 2023 21:09 — with GitHub Actions Inactive
@shacharPash shacharPash requested a review from chayim May 10, 2023 12:29
@@ -725,6 +725,63 @@ public async Task GetAsync()
Assert.Equal(35, people[1]!.Age);
}

[Fact]
[Trait("Category","edge")]
Copy link
Contributor

Choose a reason for hiding this comment

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

Perfect. We'll remove this once it hits a versioned docker. Similar to the skipif in other tests for other clients.

Copy link
Member

Choose a reason for hiding this comment

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

Should be in docker & the main ubuntu package - the windows image downloads the tarball directly and installs it in WSL.

Copy link
Member

Choose a reason for hiding this comment

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

There's a skip attribute that lets you skip a test you don't want to run, but it doesn't permit conditionals unfortunately, there are ways to setup some conditionals to override the behavior (several OSS projects do something of the like) but that's kind of overkill for something that needs a relatively simple filter like this.

Copy link
Member

@slorello89 slorello89 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@shacharPash shacharPash merged commit 508ca3d into master May 10, 2023
5 checks passed
@shacharPash shacharPash deleted the Issue128/JSON.MSET branch May 10, 2023 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for JSON.MSET
4 participants