-
Notifications
You must be signed in to change notification settings - Fork 1
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: Seldon rock integration #12
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Summary of changes: - Added tox.ini to test rock. - Added unit tests to test rock's integrity (passing) - Added integration tests (not passing, needs update from Seldon Core Operator integration tests)
Summary of changes: - Modified tox.ini according to code review comments. - Updated security requirements part. - Moved rock to use bare base.
Summary of changes: - Reorganized tox.ini - Updated tests to use filesystem.
Summary of changes: - Added fixture to test.
Summary of changes: - Added fixture to test.
…onical/seldonio-rocks into kf-3006-feat-seldon-rock-integration
Summary of changes: - Re-organise tox to avoid not needed microk8s setup.
Summary of changes: - Updated tox.ini to run tests.
Summary of changes: - Updated ROCK definition. - Updated tests.
…06-feat-seldon-rock-integration
DnPlas
reviewed
Jun 5, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @i-chvets, some comments.
CheckRock test class: canonical/charmed-kubeflow-chisme#56 |
Summary of changes: - Added test to verify that service name in ROCK matches the service name in the corresponding charm.
@DnPlas @kimwnasptd Let's resolve/approve this issue. |
i-chvets
changed the title
Kf 3006 feat seldon rock integration
feat: Seldon rock integration
Jun 14, 2023
canonical/seldon-core-operator#133 Summary of changes: - Updated rockcraft.yaml with new run-user option to run as non-root. - Updated import procedure.
Summary of changes: - Reverted to use of shell commands and `yq` instead of CheckRock test class from chisme package. NOTE: Use of bash shell commands significantly reduces maintability of tox.ini
@DnPlas @kimwnasptd Reverted to use of bash shell sripting and |
ca-scribner
previously approved these changes
Jun 15, 2023
ca-scribner
approved these changes
Jun 15, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
NOTE: This PR description only contains information related to code changes and testing, if required. All detailed information about problem statement, design, implementation, technical discussions, etc. is tracked in corresponding Github issue indicated below. This will ensure that important information is reliably recorded and tracked and not scattered across PR(s).
Details
Refer to the following Github issue for more information on feature/fix that this PR is related to:
canonical/seldon-core-operator#133
Summary of changes:
- Updated ROCK definition.
- Updated tests.
- Changed service in ROCK to match service that is launched via Charm.
- Use bash shell and
yq
instead of CheckRock test class due to size ofcharmed-kubeflow-chisme
package of 200KB.NOTE: Use of bash shell commands significantly reduces maintability of tox.ini ACK: @kimwnasptd