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

feat: mlserver sklearn rock #21

Merged
merged 3 commits into from
Jul 6, 2023
Merged

Conversation

i-chvets
Copy link
Contributor

@i-chvets i-chvets commented Jun 30, 2023

Details are in canonical/seldon-core-operator#133

Summary of changes:

  • Created rockcraft file for mlserver-sklearn rock according to best practices spec.
  • Added unit test.
  • Added tox.ini

NOTE: Only limited manual testing was performed as per spec. When integrating with correspoding charm more testing should be peformed via integration tests of the charm.

Ivan Chvets added 2 commits June 30, 2023 12:00
canonical/seldon-core-operator#133

Summary of changes:
- Created rockcraft file for mlsever-sklearn rock.
- Added unit test.
- Added tox.ini
Summary of changes:
- Use variable ROCK_RUNTIME to simplify development.
@i-chvets i-chvets requested a review from a team as a code owner June 30, 2023 19:46
@i-chvets i-chvets changed the title Kf 2273 feat mlserver sklearn rock feat: mlserver sklearn rock Jun 30, 2023
Summary of changes:
- Modified rockcraft.yaml to use environment variables in services
  instead of calling export in command line.
Copy link
Contributor

@ca-scribner ca-scribner left a comment

Choose a reason for hiding this comment

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

lgtm. Rock builds and you can exec the service, although with pebble the image is not executed quite the same way so seldon cannot actually use this image right now. That was out of scope for this task though.

@i-chvets i-chvets merged commit 99f993d into main Jul 6, 2023
@i-chvets i-chvets deleted the kf-2273-feat-mlserver-sklearn-rock branch July 6, 2023 18:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants