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

Rewrite kfp-profile-controller to use Decorator controller #334

Closed
misohu opened this issue Sep 22, 2023 · 2 comments
Closed

Rewrite kfp-profile-controller to use Decorator controller #334

misohu opened this issue Sep 22, 2023 · 2 comments

Comments

@misohu
Copy link
Member

misohu commented Sep 22, 2023

Bug Description

Right now kfp-profile-controller uses CompositeController as it is written for metacontroller-operator image version v2.0.4. This causes problem to resource-dispatcher as it uses DecoratroController written for metacontroller-operator image version v3.0.0+ (issue).

We should rewrite kfp-profile-controller to use DecoratroController so we can update metacontroller-operator image version to v3.0.0+. More about why do we have to change to DecoratroController is described in this closed issue.

In order to use DecoratroController in kfp-profile-controller we need to rewrite files/upstream/sync.py file. We can take inspiration from resource-dispatcher-image source code.

To Reproduce

None

Environment

None

Relevant log output

None

Additional context

No response

@misohu
Copy link
Member Author

misohu commented Oct 2, 2023

Fixed in #342.

Ideas for improvements
Currently we use sync.py file to specify the webhook's API. we push this file to the charm which makes it problematic:

  • we have everything in one file
  • we cannot have separate dependencies for the sync.py itself as it uses charm's one all the time
  • we are not testing the API with unit tests we just relly on integration tests for charm

The profile-controller itself has the same structure as resource-dispatcher . In there we decided to create a separate image for the resource dispatcher api, instead of pushing file to the charm. This allow us to resolve all the problems above. We can use the same approach fot the profiles controller. We need to keep in mind that upstream is organizing code in single sync.py file tho.

@NohaIhab
Copy link
Contributor

closed by #344

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

No branches or pull requests

2 participants