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

Add support for setting default value to the configuration #698

Merged
merged 3 commits into from
Apr 2, 2019
Merged

Add support for setting default value to the configuration #698

merged 3 commits into from
Apr 2, 2019

Conversation

hex108
Copy link
Contributor

@hex108 hex108 commented Apr 2, 2019

What this PR does / why we need it:
Add support for setting default value to the configuration

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #695

Special notes for your reviewer:

Release note:

Add support for setting default value to the configuration

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 2, 2019
@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 2, 2019
@hex108
Copy link
Contributor Author

hex108 commented Apr 2, 2019

/assign @k82cn

@hex108
Copy link
Contributor Author

hex108 commented Apr 2, 2019

@k82cn The test is happy now. PTAL.

k82cn
k82cn reviewed Apr 2, 2019
View reviewed changes
pkg/scheduler/plugins/conformance/conformance.go Outdated
"github.com/kubernetes-sigs/kube-batch/pkg/scheduler/framework"
)

const name = "conformance"

func init() {
Copy link
Contributor

Choose a reason for hiding this comment

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

please do not modify plugins one by one, that's ok to set default directly after we load config.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I do not catch it. How to set default directly?

k82cn
k82cn reviewed Apr 2, 2019
View reviewed changes
pkg/scheduler/framework/plugins.go Outdated
@@ -47,6 +51,39 @@ func GetPluginBuilder(name string) (PluginBuilder, bool) {
return pb, found
}

var pluginConfMutex sync.Mutex
Copy link
Contributor

Choose a reason for hiding this comment

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

I means just set default value of each config to make it simple :)

@hex108
Copy link
Contributor Author

hex108 commented Apr 2, 2019

@k82cn All problems addressed. PTAL.

@hex108
Copy link
Contributor Author

hex108 commented Apr 2, 2019

@k82cn Addressed. PTAL.

k82cn
k82cn reviewed Apr 2, 2019
View reviewed changes
Copy link
Contributor

@k82cn k82cn 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 k8s-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 2, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hex108, k82cn

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 added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 2, 2019
@k8s-ci-robot k8s-ci-robot merged commit 0b0cf5c into kubernetes-retired:master Apr 2, 2019
@hex108 hex108 deleted the plugin branch April 2, 2019 12:17
kevin-wangzefeng pushed a commit to kevin-wangzefeng/scheduler that referenced this pull request Jun 28, 2019
@k8s-ci-robot
Merge pull request kubernetes-retired#698 from hex108/plugin
3644240 
Add support for setting default value to the configuration
kevin-wangzefeng pushed a commit to kevin-wangzefeng/scheduler that referenced this pull request Jun 28, 2019
@k8s-ci-robot
Merge pull request kubernetes-retired#698 from hex108/plugin
58c66cf 
Add support for setting default value to the configuration
kevin-wangzefeng pushed a commit to kevin-wangzefeng/scheduler that referenced this pull request Jun 28, 2019
@k8s-ci-robot
Merge pull request kubernetes-retired#698 from hex108/plugin
aedb22e 
Add support for setting default value to the configuration
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@k82cn k82cn k82cn left review comments

@suleisl2000 suleisl2000 Awaiting requested review from suleisl2000

@tizhou86 tizhou86 Awaiting requested review from tizhou86

Assignees

@k82cn k82cn

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support set default value to the configuration
3 participants
@hex108 @k8s-ci-robot @k82cn