Skip to content

Standardized GET (TENSORGET,MODELGET,SCRIPTGET) methods replies (breaking change for clients) #332

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

Merged
merged 4 commits into from
Apr 22, 2020

Conversation

lantiga
Copy link
Contributor

@lantiga lantiga commented Apr 19, 2020

Addresses #329

Specifically, with this PR, get methods have a uniform signature and behavior

TENSORGET key [META] [BLOB | VALUES]
MODELGET key [META] [BLOB]
SCRIPTGET key [META] [SOURCE]

If META is not specified, all methods return a raw binary BLOB, an array VALUES or a raw SOURCE string. This helps, for instance, with dumping a buffer out of a model or a tensor from the CLI.

If only META is specified, all methods return meta data in [key, val, key, val] form. This is a breaking change with respect to the previous behavior (TENSORGET would return [dtype, shape, blob] and not ['dtype', dtype, 'shape', shape, 'blob', blob]), so THIS CHANGE WILL BREAK CLIENTS AND USER CODE.

However, this is our last call to make the API uniform and ready for RESP3.

Other notable changes:

  • keys in [key, value, ...] responses are now lowercase (including AI.INFO)
  • all commands returning strings other than "OK" or "ERR" now return bulk strings
  • the experimental _MODELLIST command has been renamed to _MODELSCAN (to prepare for the upcoming TENSORLIST data type)

Tests and docs have been updated.

@lantiga lantiga requested review from hhsecond and itamarhaber April 19, 2020 15:40
@codecov
Copy link

codecov bot commented Apr 19, 2020

Codecov Report

Merging #332 into master will decrease coverage by 0.00%.
The diff coverage is 73.68%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #332      +/-   ##
==========================================
- Coverage   56.75%   56.74%   -0.01%     
==========================================
  Files          27       27              
  Lines        5050     5107      +57     
==========================================
+ Hits         2866     2898      +32     
- Misses       2184     2209      +25     
Impacted Files Coverage Δ
src/redisai.c 76.05% <73.68%> (-1.08%) ⬇️

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 6f36dab...36cd844. Read the comment docs.

@lantiga
Copy link
Contributor Author

lantiga commented Apr 19, 2020

Genius here forgot to merge origin/master prior to creating patch. Re-pushing soon.

@lantiga
Copy link
Contributor Author

lantiga commented Apr 19, 2020

Ok, rebased on master

@lantiga lantiga merged commit 56e67de into master Apr 22, 2020
@itamarhaber itamarhaber mentioned this pull request Apr 22, 2020
28 tasks
@filipecosta90 filipecosta90 changed the title Revamp GET methods (breaking change for clients) Standardized GET (TENSORGET,MODELGET,SCRIPTGET) methods replies (breaking change for clients) May 1, 2020
@filipecosta90 filipecosta90 deleted the get_revamp branch May 1, 2020 10:35
lantiga added a commit that referenced this pull request May 6, 2020
 
Revamp GET methods (breaking change for clients) (#332)

* Revamp GET methods (breaking change for clients)

* Fix function name after rebase

* Fix DAG tests after rebase

* Remove outdated comment
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.

2 participants