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

fix potential bug on ts.add and ts.madd, when there is specific type … #1503

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

Conversation

OfirMos
Copy link
Contributor

@OfirMos OfirMos commented Aug 10, 2023

…of errors the opened key isn't being closed
Currently there is no bug ,since RedisModule_AutoMemory is used and the key is anyway being closed, but in the future, this command would be ready to drop auto memory.

@OfirMos OfirMos requested a review from oshadmi August 10, 2023 06:57
@OfirMos OfirMos requested a review from danni-m as a code owner August 10, 2023 06:57
@sonatype-lift
Copy link

sonatype-lift bot commented Aug 10, 2023

Sonatype Lift is retiring

Sonatype Lift will be retiring on Sep 12, 2023, with its analysis stopping on Aug 12, 2023. We understand that this news may come as a disappointment, and Sonatype is committed to helping you transition off it seamlessly. If you’d like to retain your data, please export your issues from the web console.
We are extremely grateful and thank you for your support over the years.

📖 Read about the impacts and timeline

@OfirMos OfirMos enabled auto-merge (rebase) August 10, 2023 06:57
@@ -279,4 +279,26 @@ def test_ts_upsert_bug():
res2 = r.execute_command("ts.range", t1, first_chunk_last_ts, first_chunk_last_ts + 100)
info2 = TSInfo(r.execute_command("ts.info", t1, 'DEBUG'))
assert res2 == [res[0], [first_chunk_last_ts + 100, first_chunk_last_val]]

def test_error():
Copy link

Choose a reason for hiding this comment

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

7% of developers fix this issue

E302: expected 2 blank lines, found 1


ℹ️ Expand to see all @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.

…of errors the opened key isn't being closed
@codecov
Copy link

codecov bot commented Aug 10, 2023

Codecov Report

Patch coverage: 88.88% and project coverage change: +0.03% 🎉

Comparison is base (7e8f3e1) 88.99% compared to head (7772857) 89.03%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1503      +/-   ##
==========================================
+ Coverage   88.99%   89.03%   +0.03%     
==========================================
  Files          35       35              
  Lines        5834     5844      +10     
==========================================
+ Hits         5192     5203      +11     
+ Misses        642      641       -1     
Files Changed Coverage Δ
src/module.c 86.62% <88.88%> (+0.04%) ⬆️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@oshadmi oshadmi left a comment

Choose a reason for hiding this comment

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

Few comments.
Please also add to the description, as discussed, that currently there is no bug ,since RedisModule_AutoMemory is used and the key is anyway being closed, but in the future, this command would be ready to drop auto memory.

tests/flow/test_ts_add.py Show resolved Hide resolved
tests/flow/test_ts_add.py Show resolved Hide resolved
tests/flow/test_ts_add.py Show resolved Hide resolved
tests/flow/test_ts_add.py Outdated Show resolved Hide resolved
src/module.c Show resolved Hide resolved
@RedisTimeSeries RedisTimeSeries deleted a comment from sonatype-lift bot Aug 16, 2023
@RedisTimeSeries RedisTimeSeries deleted a comment from sonatype-lift bot Aug 16, 2023
OfirMos and others added 2 commits August 16, 2023 10:01
Co-authored-by: Omer Shadmi <76992134+oshadmi@users.noreply.github.com>
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.

None yet

2 participants