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: kfp-viz sidecar rewrite with base charm #309

Merged
merged 2 commits into from
Aug 24, 2023

Conversation

NohaIhab
Copy link
Contributor

based on #305 due to unverified commits

Rewrite the kfp-viz charm, migrating it from podspec to sidecar using the reusable base charm in charmed-kubeflow-chisme. More details on the base charm in this README.md

Summary of changes

  • rewrites all charm logic in terms of Components
  • implements the Pebble container
  • implements the relation handler
  • refactors the unit tests to use the base charm tooling
  • modifies requirements.in and requirements-unit.in and updates the requirements-*.txt's

NOTE: Due to deployment parameter in PodSpec version, kfp-persistence cannot be upgraded using Juju refresh:
juju.errors.JujuAPIError: Juju on containers does not support updating deployment info for services.
Upgrade will be done by re-deploying the charm.

@i-chvets
Copy link
Contributor

i-chvets commented Aug 24, 2023

Confirming that all comments, reviews, and conversation resolutions from #305 gold true for this PR.
Re-ran unit and integration test locally as well.

@i-chvets
Copy link
Contributor

@NohaIhab We need to update bunde definition to include trust: https://github.com/canonical/bundle-kubeflow/blob/main/releases/latest/edge/bundle.yaml#L120

This PR LGTM. Approving.

Copy link
Contributor

@i-chvets i-chvets left a comment

Choose a reason for hiding this comment

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

Appoved based on reviews/discussions in #305

@i-chvets i-chvets merged commit 38fd0f0 into main Aug 24, 2023
@i-chvets i-chvets deleted the kf-4073-kfp-viz-sidecar-base-charm branch August 24, 2023 19:44
i-chvets pushed a commit to canonical/bundle-kubeflow that referenced this pull request Aug 24, 2023
Summary of changes:
- Added trust to kfp-viz to support newly re-written charm canonical/kfp-operators#309
NohaIhab added a commit to canonical/bundle-kubeflow that referenced this pull request Aug 25, 2023
* fix: added trust to kfp-viz to support new charm

Summary of changes:
- Added trust to kfp-viz to support newly re-written charm canonical/kfp-operators#309

---------

Co-authored-by: Noha Ihab <49988746+NohaIhab@users.noreply.github.com>
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.

2 participants