Skip to content
This repository has been archived by the owner on Sep 12, 2023. It is now read-only.

Provides more flexibility for customizations #91

Merged
merged 2 commits into from
May 16, 2020

Conversation

Jeffwan
Copy link
Member

@Jeffwan Jeffwan commented May 16, 2020

Following methods both have common implementation and user can override them if existing solution doesn't fit.

  1. Add GetPodsForJob and GetServiceForJob back for kubebuilder based operator.
  2. Add ReconcileJobs, ReconcilePods and ReconcileServices into interface.
  3. Expose methods for better customization

This has been tested with mxnet-operator (built with low level informer and lister) and xgboost-operator (built with kubebuilder)

Seems tensorflow operator may have some customizations on reconcile logic, they can override the methods and still leverage some common methods.

1. Move ReconcileJobs, ReconcilePods, ReconcileServices to interface. JobController provides common implementation. User doesn’t necessarily implement these methods a
nd only user facing change is user need to embed JobController. If operators has custom logic, they can override these reconciling logic

2. Expose few private methods and when user want to have some customization, they can reuse those method
- DeletePodsAndServices
- PastBackoffLimit
- PastActiveDeadline
- CleanupJob
1. JobController has default implementation for GetPodsForJob and GetServicesForJob. User needs to pass a PodLister and SeviceLister. For operators using informers, user don’t need to make any change.

2. However, the challenge part if we have more operators are built with kubebuilder which use higher level APIs and doesn’t create `SharedInformerFactory`. In order to accomendate these cases, we expose methods and user can implement them using latest  reconciler.Client.List().

Signed-off-by: Jiaxin Shan <seedjeffwan@gmail.com>
@kubeflow-bot
Copy link

This change is Reviewable

Copy link
Member

@terrytangyuan terrytangyuan left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@k8s-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: terrytangyuan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit 4d01e68 into kubeflow:master May 16, 2020
@Jeffwan Jeffwan deleted the support_multi_builder branch May 16, 2020 23:09
georgkaleido pushed a commit to georgkaleido/common that referenced this pull request Jun 9, 2022
…es (kubeflow#91)

* Add option to fetch full resolution samples

* Add logs

* Add script to perform time and accuracy evaluation

* Add script to compute discrepancy

* Add logging

* Add field "rgb_preview" in SmartAlphaImage

* Add utility

* Improve trimap performance script

* Prevent download of already existing samples

* Add mIoU accuracy

* Add output to json file

* Add output to json file

* Create custom function to read image and perform an additional image downscaling specifically targeted to compute the trimap

* Set trimap_size as a member

* Add trimap_size as an argument

* Add trimap_size as an argument

* Restore full processing of removebg output, to compare MSE of alpha channel

* Fix missing alpha gt

* Refactor handling of megapixel_limit

* Try argument "reducing_gap" in Pillow.Image.resize

* in read_image(), change the way to resize the output optimized for trimap

* Add image for trimap in removebg-server.py

* wip evaluation

* Move im_for_trimap in a unique block

* Remove duplicate MSE

* Restore megapixel_limit_trimap to 0.25 instead of 1.0

* Add MSE in the performance compilation

* Change megapixel limit for trimap

* Change downscale behavior based on scale_factor (BOX or BICUBIC)

* Add a megapixel limit when downscaling with BICUBIC

* Add an option to grab file4m from Danni

* Update requirements

* Fix case where input image has 4 channels
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants