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.

Sign up for GitHub

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

Jump to bottom

feat: add multiple save for redis registry #281

Merged
merged 18 commits into from
Sep 18, 2023
Merged

feat: add multiple save for redis registry #281

merged 18 commits into from
Sep 18, 2023

Conversation

s0nicboOm
Copy link
Contributor

@s0nicboOm s0nicboOm commented Sep 9, 2023 •
edited
Loading

  1. Option to multisave in redis registry
  2. support caching for version calls
  3. In udfs/ load models using version call instead of latest calls

@codecov
Copy link

codecov bot commented Sep 12, 2023 •
edited
Loading

Codecov Report

Merging #281 (909a19a) into main (a364721) will increase coverage by 0.24%.
The diff coverage is 95.60%.

@@            Coverage Diff             @@
##             main     #281      +/-   ##
==========================================
+ Coverage   92.41%   92.66%   +0.24%     
==========================================
  Files          74       74              
  Lines        3349     3380      +31     
  Branches      267      275       +8     
==========================================
+ Hits         3095     3132      +37     
+ Misses        201      196       -5     
+ Partials       53       52       -1
Files Changed Coverage Δ
numalogic/udfs/trainer.py 90.15% <78.57%> (+0.75%) ⬆️
numalogic/registry/redis_registry.py 97.41% <97.72%> (-0.28%) ⬇️
numalogic/connectors/druid.py 86.36% <100.00%> (ø)
numalogic/tools/types.py 93.54% <100.00%> (+0.69%) ⬆️
numalogic/udfs/inference.py 94.44% <100.00%> (-0.68%) ⬇️
numalogic/udfs/postprocess.py 86.48% <100.00%> (+0.18%) ⬆️
numalogic/udfs/preprocess.py 87.80% <100.00%> (+10.02%) ⬆️
numalogic/udfs/tools.py 100.00% <100.00%> (ø)

... and 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@s0nicboOm
fix: test cases
965c9c5 
Signed-off-by: s0nicboOm <[email protected]>
@s0nicboOm
fix: update version
b230ce8 
Signed-off-by: s0nicboOm <[email protected]>
@s0nicboOm
update workflows
d00258f 
Signed-off-by: s0nicboOm <[email protected]>
@s0nicboOm
fix: update version
ba9372d 
Signed-off-by: s0nicboOm <[email protected]>
@s0nicboOm
fix: mlflow test case
b30b808 
Signed-off-by: s0nicboOm <[email protected]>
@s0nicboOm s0nicboOm marked this pull request as ready for review September 13, 2023 02:03
ab93
ab93 requested changes Sep 13, 2023
View reviewed changes
Copy link
Member

@ab93 ab93 left a comment

Choose a reason for hiding this comment

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

Some comments

numalogic/config/_config.py Outdated Show resolved Hide resolved
numalogic/connectors/druid.py Show resolved Hide resolved
numalogic/udfs/entities.py Outdated Show resolved Hide resolved
numalogic/udfs/__main__.py Outdated Show resolved Hide resolved
nkoppisetty
nkoppisetty requested changes Sep 13, 2023
View reviewed changes
Copy link
Member

@nkoppisetty nkoppisetty left a comment

Choose a reason for hiding this comment

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

couple of comments, and also you need to rebase.

numalogic/udfs/tools.py Outdated Show resolved Hide resolved
tests/udfs/test_inference.py Show resolved Hide resolved
@s0nicboOm s0nicboOm self-assigned this Sep 14, 2023
ab93
ab93 reviewed Sep 14, 2023
View reviewed changes
Copy link
Member

@ab93 ab93 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

numalogic/registry/redis_registry.py Outdated Show resolved Hide resolved
numalogic/registry/redis_registry.py Outdated Show resolved Hide resolved
numalogic/tools/types.py Outdated Show resolved Hide resolved
numalogic/registry/redis_registry.py Show resolved Hide resolved
numalogic/registry/redis_registry.py Show resolved Hide resolved
numalogic/registry/redis_registry.py Outdated Show resolved Hide resolved
numalogic/udfs/tools.py Show resolved Hide resolved
ab93
ab93 approved these changes Sep 14, 2023
View reviewed changes
Copy link
Member

@ab93 ab93 left a comment

Choose a reason for hiding this comment

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

Please rebase before merging

@s0nicboOm
Merge branch 'main' into multiple_save
d1789c0 
# Conflicts:
#	numalogic/udfs/__main__.py
#	numalogic/udfs/inference.py
#	numalogic/udfs/trainer.py
#	poetry.lock
#	pyproject.toml
@s0nicboOm
fix: update mlflow version
d20b1e3 
Signed-off-by: s0nicboOm <[email protected]>
@s0nicboOm
fix: remove unwanted files
909a19a 
Signed-off-by: s0nicboOm <[email protected]>
@s0nicboOm s0nicboOm enabled auto-merge (squash) September 17, 2023 02:11
nkoppisetty
nkoppisetty approved these changes Sep 18, 2023
View reviewed changes
Copy link
Member

@nkoppisetty nkoppisetty left a comment

Choose a reason for hiding this comment

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

LGTM

@s0nicboOm s0nicboOm merged commit bc1c627 into numaproj:main Sep 18, 2023
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nkoppisetty nkoppisetty nkoppisetty approved these changes

@ab93 ab93 ab93 approved these changes

@qhuai qhuai Awaiting requested review from qhuai qhuai is a code owner

Assignees

@s0nicboOm s0nicboOm

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@s0nicboOm @ab93 @nkoppisetty