Skip to content

Conversation

@GuyAv46
Copy link
Contributor

@GuyAv46 GuyAv46 commented Nov 28, 2021

No description provided.

@codecov
Copy link

codecov bot commented Nov 28, 2021

Codecov Report

Merging #883 (bc2a07d) into master (0390451) will increase coverage by 0.09%.
The diff coverage is 85.10%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #883      +/-   ##
==========================================
+ Coverage   81.21%   81.30%   +0.09%     
==========================================
  Files          54       54              
  Lines        8145     8179      +34     
==========================================
+ Hits         6615     6650      +35     
+ Misses       1530     1529       -1     
Impacted Files Coverage Δ
src/backends/util.c 33.33% <30.00%> (-5.13%) ⬇️
src/execution/DAG/dag.c 91.75% <100.00%> (+0.02%) ⬆️
src/execution/command_parser.c 92.45% <100.00%> (+0.29%) ⬆️
src/redis_ai_objects/tensor.c 91.97% <100.00%> (+0.04%) ⬆️
src/redisai.c 87.79% <100.00%> (+0.42%) ⬆️
src/redismodule.h 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0aa5085...bc2a07d. Read the comment docs.

@GuyAv46 GuyAv46 added ci-test and removed ci-test labels Nov 28, 2021
Copy link

@DvirDukhan DvirDukhan left a comment

Choose a reason for hiding this comment

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

looks very good!
please note the comments on the python file

@GuyAv46 GuyAv46 requested a review from DvirDukhan November 30, 2021 13:19
@GuyAv46 GuyAv46 removed the ci-test label Dec 1, 2021
and updated redismodule.h file
for python <3.7 compatibility
search for the command in the last 10 commands
improved test_slowlog_time_dag_modelexecute_financialNet_autobatch test
RedisModule_BlockedClientMeasureTime functions
(supported from version 6.2)
Added 'patch' argument for skip_if_not_version command.

Added comments in
test_slowlog_time_dag_modelexecute_financialNet_autobatch test.
until additional support will be added
@GuyAv46 GuyAv46 merged commit ff347c6 into master Dec 1, 2021
@GuyAv46 GuyAv46 deleted the slowlog_impl branch December 1, 2021 19:42
GuyAv46 added a commit that referenced this pull request Dec 2, 2021
* added slowlog timer implementation
and updated redismodule.h file

* added test for slowlog timer implementation

* replaced time_ns() with time()
for python <3.7 compatibility

* splited asserts for better error catching

* instead of assuming last command was DAGEXECUTE,
search for the command in the last 10 commands

* Added a wrapper for skipping test by redis version
improved test_slowlog_time_dag_modelexecute_financialNet_autobatch test

* added a check for version before using
RedisModule_BlockedClientMeasureTime functions
(supported from version 6.2)

* Review fixes:

Added 'patch' argument for skip_if_not_version command.

Added comments in
test_slowlog_time_dag_modelexecute_financialNet_autobatch test.

* Valgring test will run on 6.0 redis,
until additional support will be added
GuyAv46 added a commit that referenced this pull request Dec 2, 2021
* Slowlog implementation (#883)

* added slowlog timer implementation
and updated redismodule.h file

* added test for slowlog timer implementation

* replaced time_ns() with time()
for python <3.7 compatibility

* splited asserts for better error catching

* instead of assuming last command was DAGEXECUTE,
search for the command in the last 10 commands

* Added a wrapper for skipping test by redis version
improved test_slowlog_time_dag_modelexecute_financialNet_autobatch test

* added a check for version before using
RedisModule_BlockedClientMeasureTime functions
(supported from version 6.2)

* Review fixes:

Added 'patch' argument for skip_if_not_version command.

Added comments in
test_slowlog_time_dag_modelexecute_financialNet_autobatch test.

* Valgring test will run on 6.0 redis,
until additional support will be added

* Slowlog impl fix (#885)

* fix for circleci valgrind-cluster test:
uses 6.2.5-x64-bullseye again

* fix the debug messege:
will display the test-function name now

* update valgrind test to 6.2
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.

3 participants