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

Add accessUserInformation to the plugin security policy. (Issue # 844) #1598

Merged
merged 3 commits into from
Jan 30, 2024

Conversation

austintlee
Copy link
Collaborator

Description

Fixes an issue where zip files in model_cache are not getting deleted.

Issues Resolved

#844

Check List

  • New functionality includes testing.
    • [x ] All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • [ x] Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Copy link

codecov bot commented Nov 5, 2023

Codecov Report

Merging #1598 (047017a) into main (66d4cd4) will decrease coverage by 0.05%.
Report is 14 commits behind head on main.
The diff coverage is n/a.

@@             Coverage Diff              @@
##               main    #1598      +/-   ##
============================================
- Coverage     78.35%   78.31%   -0.05%     
+ Complexity     2343     2341       -2     
============================================
  Files           194      194              
  Lines          9528     9528              
  Branches        953      953              
============================================
- Hits           7466     7462       -4     
- Misses         1633     1636       +3     
- Partials        429      430       +1     
Flag Coverage Δ
ml-commons 78.31% <ø> (-0.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 2 files with indirect coverage changes

@dhrubo-os
Copy link
Collaborator

Awesome!! If you think the issue is resolved, can we remove this check here?

@austintlee
Copy link
Collaborator Author

@dhrubo-os I commented it out and the test is passing on my machine (Mac M2):

> Task :opensearch-ml-plugin:integTest

org.opensearch.ml.rest.RestMLDeployModelActionIT > testReDeployModel STANDARD_OUT
    [2023-11-05T02:20:47,690][INFO ][o.o.m.r.RestMLDeployModelActionIT] [testReDeployModel] before test
    [2023-11-05T02:20:47,782][INFO ][o.o.m.r.RestMLDeployModelActionIT] [testReDeployModel] initializing REST clients against [http://[::1]:57407, http://127.0.0.1:57408]
    [2023-11-05T02:21:07,048][INFO ][o.o.m.r.RestMLDeployModelActionIT] [testReDeployModel] Re-Deploy model {model_id=qSztnYsBgakGMWcWLqkT, task_type=DEPLOY_MODEL, function_name=TEXT_EMBEDDING, state=CREATED, worker_node=[QWNvoxAmSAmzfjihO8rmzg], create_time=1.699161666982E12, last_update_time=1.699161666982E12, is_async=true}
    [2023-11-05T02:21:07,225][INFO ][o.o.m.r.RestMLDeployModelActionIT] [testReDeployModel] Get Model after re-deploy {name=test_model_name, model_group_id=pyztnYsBgakGMWcWLKnJ, algorithm=TEXT_EMBEDDING, model_version=1, model_format=TORCH_SCRIPT, model_state=DEPLOYED, model_content_size_in_bytes=4554671.0, model_content_hash_value=e13b74006290a9d0f58c1376f9629d4ebc05a0f9385f40db837452b167ae9021, model_config={model_type=bert, embedding_dimension=768.0, framework_type=SENTENCE_TRANSFORMERS}, created_time=1.69916164855E12, last_updated_time=1.699161667177E12, last_registered_time=1.699161650625E12, last_deployed_time=1.699161667176E12, total_chunks=1.0, planning_worker_node_count=1.0, current_worker_node_count=1.0, planning_worker_nodes=[QWNvoxAmSAmzfjihO8rmzg], deploy_to_all_nodes=true}
    [2023-11-05T02:21:07,423][INFO ][o.o.m.r.RestMLDeployModelActionIT] [testReDeployModel] after test

Signed-off-by: Austin Lee <[email protected]>
@austintlee
Copy link
Collaborator Author

@dhrubo-os @zane-neo can you guys take a look?

@dhrubo-os
Copy link
Collaborator

@dhrubo-os @zane-neo can you guys take a look?

Looks good to me. I'm not sure why it's showing java 20 test in the workflow as we updated to java 21. Could you please take all the updates from main? Thanks.

@dhrubo-os
Copy link
Collaborator

@austintlee while you are working on this, could you please test this redeploy model in your end with java 21? For java 21, sometiimes it's showing timeout issue.

@dhrubo-os dhrubo-os mentioned this pull request Dec 6, 2023
5 tasks
@ylwu-amzn ylwu-amzn merged commit ee69eb0 into opensearch-project:main Jan 30, 2024
9 of 13 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jan 30, 2024
#1598)

* Add accessUserInformation to the plugin security policy.  (Issue # 844)

Signed-off-by: Austin Lee <[email protected]>

* Re-enable ReDeploy IT test.

Signed-off-by: Austin Lee <[email protected]>

* Apply spotless.

Signed-off-by: Austin Lee <[email protected]>

---------

Signed-off-by: Austin Lee <[email protected]>
(cherry picked from commit ee69eb0)
ylwu-amzn pushed a commit that referenced this pull request Feb 1, 2024
#1598) (#1959)

* Add accessUserInformation to the plugin security policy.  (Issue # 844)

Signed-off-by: Austin Lee <[email protected]>

* Re-enable ReDeploy IT test.

Signed-off-by: Austin Lee <[email protected]>

* Apply spotless.

Signed-off-by: Austin Lee <[email protected]>

---------

Signed-off-by: Austin Lee <[email protected]>
(cherry picked from commit ee69eb0)

Co-authored-by: Austin Lee <[email protected]>
austintlee added a commit to austintlee/ml-commons that referenced this pull request Mar 19, 2024
opensearch-project#1598)

* Add accessUserInformation to the plugin security policy.  (Issue # 844)

Signed-off-by: Austin Lee <[email protected]>

* Re-enable ReDeploy IT test.

Signed-off-by: Austin Lee <[email protected]>

* Apply spotless.

Signed-off-by: Austin Lee <[email protected]>

---------

Signed-off-by: Austin Lee <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants