Skip to content
This repository was archived by the owner on Apr 15, 2024. It is now read-only.

Conversation

@laguirre-cs
Copy link
Contributor

@laguirre-cs laguirre-cs commented Jun 15, 2023

Issue: https://github.com/CognitiveScale/certifai/issues/4812

Overview of changes:

  • Fixes broken link to German Credit UCI Dataset - the content was moved. (Fixes pipeline failures)
  • Adds Python based unit tests for running the Prediction Service examples.
  • Update run_test.sh script, for running containerized model examples locally while skipping h2o models
  • Update example Scan Manager config file to remove scikit0.23 reference - replace with python
  • Bump xgboost from 1.2.0 to 1.7.2 - this was required to be able to install xgboost on the M1 Mac (v1.2.0 was released in 2020!!)

…em. Requires running apps WITHOUT gunicorn to avoid zombies
…ing exampels, fix broken link, remove beta flag warning
@laguirre-cs laguirre-cs marked this pull request as ready for review June 16, 2023 15:16
@laguirre-cs laguirre-cs requested a review from mchutani-CS June 16, 2023 15:17
Comment on lines +92 to +96
# WARNING: Killing the subprocess may not kill any workers spawned by the process (e.g. gunicorn!)
p.kill()
p.wait()
capture_output(stdout.name, stderr.name)
raise
Copy link
Contributor Author

@laguirre-cs laguirre-cs Jun 20, 2023

Choose a reason for hiding this comment

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

The shutdown functionality for the prediction services is kinda wonky right now, especially when testing. It seemed like the best way to handle it was by using Python's subprocess module & just killing the parent process (and for the sake of our examples, the apps run with the dev server so we never risk leaving zombie workers).

  • The builtin /shutdown endpoint only works for gunicorn servers (when production=True) - this is a known bug in the Model SDK package.
  • Not all tests will call the /shutdown endpoint - like when running python scans. Nor do we expect all the servers to be run with production=True
  • Python's kill() here will kill the process, but if you are using gunicorn, then the spawned workers are left as zombies

Comment on lines -1 to +2
scikit_0.23:
deployment: scikit_0.23_deployment.yml
python:
deployment: python_deployment.yml
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm replacing the scikit_0.23 reference with python, and adding the current set of base images. We will need to update this when the release time comes (or figure out some way to automate this).

Follow up action would be to update the copy on our Certifai Dev to have python

@laguirre-cs laguirre-cs requested a review from ssingh-CS June 21, 2023 15:30
@laguirre-cs
Copy link
Contributor Author

@laguirre-cs laguirre-cs requested a review from mcriscolo-cs June 28, 2023 13:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant