Add functionality to delete applications#457
Conversation
|
Can one of the admins verify this patch? |
|
@JayShenoy can you delete all the cmake files you committed? This PR has 13,000 files and over a million lines of code. |
|
My mistake, I just removed that mess |
|
@dcrankshaw The frontend and management tests now all pass, but the integration tests are failing because of issues with Docker that I believe are local to my machine. |
|
jenkins add to whitelist |
|
Test FAILed. |
dcrankshaw
left a comment
dcrankshaw
left a comment
There was a problem hiding this comment.
This looks great. Very nice work. I'll merge once the tests pass.
simon-mo
left a comment
simon-mo
left a comment
There was a problem hiding this comment.
@JayShenoy Great work. A quick question: does this PR intend to just delete applications? Is it suppose to do any clean up between app_model links.
For example, if I registered an application simple-app, and deploy model sum-model linking to simple-app, right now if I delete simple-app, the model container will not be deleted. This is intentional?
Thanks for the great work!
| logger.error(msg) | ||
| raise ClipperException(msg) | ||
| else: | ||
| logger.info("Application {app} was successfully deleted").format( |
There was a problem hiding this comment.
This line will error:
logger.info("Application {app} was successfully deleted").format(
AttributeError: 'NoneType' object has no attribute 'format'
| default_output = "DEFAULT" | ||
| slo_micros = 30000 | ||
| app_name = "testapp" | ||
| self.clipper_conn.register_application(app_name, input_type, |
There was a problem hiding this comment.
This line will error:
Traceback (most recent call last):
File "clipper_admin_tests.py", line 88, in test_delete_application_correct
self.clipper_conn.register_application(app_name, input_type,
NameError: global name 'input_type' is not defined
| self.clipper_conn.link_model_to_app(app_name, not_deployed_model) | ||
| self.assertTrue("No model with name" in str(context.exception)) | ||
|
|
||
| def test_delete_application_correct(self): |
There was a problem hiding this comment.
You also need to add this test function to SHORT_TEST_ORDERING list below:
|
@simon-mo Yeah that behavior is correct. Deleting an application should not delete any models. |
|
Test FAILed. |
|
@simon-mo This PR is purely intended to delete applications. I believe someone else is working on deleting models. @dcrankshaw I have made the necessary revisions. |
|
Test FAILed. |
|
Test FAILed. |
|
Test PASSed. |
|
Bless up 🙏 |
|
Nice job! |
4 participants
No description provided.