8000 Added test cases for SageMaker MME requirements by agunapal · Pull Request #2200 · pytorch/serve · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
/ serve Public
< 8000 div id="repository-details-container" class="flex-shrink-0" data-turbo-replace style="max-width: 70%;">

Added test cases for SageMaker MME requirements #2200

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
Merged
merged 37 commits into from
Apr 4, 2023
Merged

Added test cases for SageMaker MME requirements #2200

merged 37 commits into from
Apr 4, 2023

Conversation

agunapal
Copy link
Collaborator
@agunapal agunapal commented Mar 24, 2023 •
edited
Loading

Description

Added 2 test cases for SageMaker MME requirements

  • Model not loaded (404)
  • OOM Error (507

APIs defined here

Fixes #(issue)

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

Feature/Issue validation/testing

Please describe the Unit or Integration tests that you ran to verify your changes and relevant result summary. Provide instructions so it can be reproduced.
Please also list any relevant details for your test configuration.

  • PyTest
python -m pytest -v test_sm_mme_requirements.py 
============================================================================= test session starts =============================================================================
platform linux -- Python 3.8.16, pytest-7.2.2, pluggy-1.0.0 -- /home/ubuntu/anaconda3/envs/torchserve/bin/python
cachedir: .pytest_cache
rootdir: /home/ubuntu/serve
plugins: cov-4.0.0, mock-3.10.0
collected 3 items                                                                                                                                                             

test_sm_mme_requirements.py::test_no_model_loaded PASSED                 [ 33%]
test_sm_mme_requirements.py::test_oom_on_model_load PASSED               [ 66%]
test_sm_mme_requirements.py::test_oom_on_invoke PASSED                   [100%]

Checklist:

  • Did you have fun?
  • Have you added tests that prove your fix is effective or that this feature works?
  • Has code been commented, particularly in hard-to-understand areas?
  • Have you made corresponding changes to the documentation?

@agunapal agunapal requested a review from lxning March 24, 2023 18:02
@codecov
Copy link
codecov bot commented Mar 24, 2023 •
edited
Loading

Codecov Report

Merging #2200 (3e86d7f) into master (98ca286) will decrease coverage by 0.16%.
The diff coverage is 0.00%.

❗ Current head 3e86d7f differs from pull request most recent head 967a5f7. Consider uploading reports for the commit 967a5f7 to get more accurate results

@@            Coverage Diff             @@
##           master    #2200      +/-   ##
==========================================
- Coverage   71.52%   71.37%   -0.16%     
==========================================
  Files          73       73              
  Lines        3329     3336       +7     
  Branches       57       57              
==========================================
  Hits         2381     2381              
- Misses        945      952       +7     
  Partials        3        3
Impacted Files Coverage Δ
ts/model_service_worker.py 65.71% <0.00%> (-1.94%) ⬇️
ts/service.py 71.60% <0.00%> (-2.76%) ⬇️

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

lxning
lxning reviewed Mar 28, 2023
View reviewed changes
@agunapal
Copy link
Collaborator Author

@lxning

Now on model loading, I am getting code 507,

but the message is as follows

{
  "code": 507,
  "type": "InternalServerException",
  "message": "Failed to start workers for model BERTSeqClassification version: 1.0"
}

This happens because here we are setting this. I don't know if we want to fix this in this PR.

@lxning
Copy link
Collaborator
lxning commented Mar 28, 2023

@agunapal reply to question:

let's add sth like this at https://github.com/pytorch/serve/blob/master/frontend/server/src/main/java/org/pytorch/serve/util/ApiUtils.java#L283:
if (v == 507) {
String msg = "outofmemory"
} else {
}

@agunapal agunapal changed the title (WIP) Added 2 test cases for SageMaker MME requirements (WIP) Added test cases for SageMaker MME requirements Mar 28, 2023
@agunapal agunapal requested a review from lxning March 28, 2023 20:30
@agunapal
Copy link
Collaborator Author

@lxning Looks like that wasn't the right place. v value was 413 there. We can perhaps address this in another PR in interest of time?

@agunapal agunapal requested review from msaroufim and lxning March 29, 2023 22:13
msaroufim
msaroufim requested changes Mar 31, 2023
View reviewed changes
@msaroufim msaroufim self-requested a review April 3, 2023 23:55
@agunapal agunapal changed the title (WIP) Added test cases for SageMaker MME requirements Added test cases for SageMaker MME requirements Apr 4, 2023
@agunapal agunapal merged commit a4a1f65 into master Apr 4, 2023
@agunapal agunapal deleted the feature/regression_test_sm_mme branch April 4, 2023 01:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@msaroufim msaroufim msaroufim approved these changes

@lxning lxning lxning approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@agunapal @lxning @msaroufim @ankithagunapal
0