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

Missing methods in reconciler.v1 #159

Open
zw0610 opened this issue Sep 6, 2021 · 1 comment
Open

Missing methods in reconciler.v1 #159

zw0610 opened this issue Sep 6, 2021 · 1 comment

Comments

@zw0610
Copy link
Member

zw0610 commented Sep 6, 2021

In #141, methods implemented as panic("implement me!") that are aimed to let developers overriding is removed as the pull request is merged. However, when creating a demo reconciler or even run the test file pkg/reconciler.v1/common/service_test.go, it raises error:

# github.com/kubeflow/common/pkg/reconciler.v1/common
pkg/reconciler.v1/common/reconciler.go:60:24: cannot use jobInter (type *KubeflowJobReconciler) as type JobInterface in assignment:
	*KubeflowJobReconciler does not implement JobInterface (missing ExtractJobStatus method)
pkg/reconciler.v1/common/reconciler.go:66:24: cannot use jobInter (type *KubeflowJobReconciler) as type JobInterface in assignment:
	*KubeflowJobReconciler does not implement JobInterface (missing ExtractJobStatus method)
pkg/reconciler.v1/common/reconciler.go:73:34: cannot use jobInter (type *KubeflowJobReconciler) as type JobInterface in assignment:
	*KubeflowJobReconciler does not implement JobInterface (missing ExtractJobStatus method)
FAIL	command-line-arguments [build failed]
FAIL

Simply comment out the DefaultKubeflowReconciler method in pkg/reconciler.v1/common/reconciler.go does not resolve the problem as new error appears:

# github.com/kubeflow/common/test_job/reconciler.v1/test_job
test_job/reconciler.v1/test_job/test_job_reconciler.go:54:24: cannot use jobInter (type *"github.com/kubeflow/common/pkg/reconciler.v1/common".KubeflowJobReconciler) as type "github.com/kubeflow/common/pkg/reconciler.v1/common".JobInterface in assignment:
	*"github.com/kubeflow/common/pkg/reconciler.v1/common".KubeflowJobReconciler does not implement "github.com/kubeflow/common/pkg/reconciler.v1/common".JobInterface (missing ExtractJobStatus method)
test_job/reconciler.v1/test_job/test_job_reconciler.go:60:24: cannot use jobInter (type *"github.com/kubeflow/common/pkg/reconciler.v1/common".KubeflowJobReconciler) as type "github.com/kubeflow/common/pkg/reconciler.v1/common".JobInterface in assignment:
	*"github.com/kubeflow/common/pkg/reconciler.v1/common".KubeflowJobReconciler does not implement "github.com/kubeflow/common/pkg/reconciler.v1/common".JobInterface (missing ExtractJobStatus method)
test_job/reconciler.v1/test_job/test_job_reconciler.go:67:34: cannot use jobInter (type *"github.com/kubeflow/common/pkg/reconciler.v1/common".KubeflowJobReconciler) as type "github.com/kubeflow/common/pkg/reconciler.v1/common".JobInterface in assignment:
	*"github.com/kubeflow/common/pkg/reconciler.v1/common".KubeflowJobReconciler does not implement "github.com/kubeflow/common/pkg/reconciler.v1/common".JobInterface (missing ExtractJobStatus method)
FAIL	command-line-arguments [build failed]
FAIL

While the error message shows the ExtractJobStatus method is missing, there are other methods still needs to be implemented.

There seems two solutions to this problem:

  1. comment out DefaultKubeflowReconciler method in pkg/reconciler.v1/common/reconciler.go and let user to implement these methods in a lower reconciler level (JobReconciler) level instead of the top level
  2. add these methods back in pkg/reconciler.v1/common with panic("implement me!") so developers only need to override these methods at the top level.
@Jeffwan
Copy link
Member

Jeffwan commented Sep 19, 2021

I think first option means we can not provide high level reconciler for users? If that's true, I prefer second option and probably give clear documents and let user know this should be overrided

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants