Skip to content
This repository has been archived by the owner on Jan 21, 2022. It is now read-only.

Add minimum_candidate_stagers to manifest template #966

Closed

Conversation

ScarletTanager
Copy link
Contributor

[#119362989]

Signed-off-by: Dan Lavine dlavine@us.ibm.com

[#119362989]

Signed-off-by: Dan Lavine <dlavine@us.ibm.com>
@cfdreddbot
Copy link

Hey ScarletTanager!

Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you and the commit authors have already signed the CLA.

@cf-gitbot
Copy link
Collaborator

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/120018459

The labels on this github issue will be updated when the story is started.

@Amit-PivotalLabs
Copy link
Contributor

Any reason this default of 5 isn't in the job spec rather than the manifest template @ScarletTanager @DanLavine?

@ghost
Copy link

ghost commented May 31, 2016

@Amit-PivotalLabs,

There is already a default of 5 in the job spec. However it seems kind of odd when looking at a manifest that was generated and this field having a nil value. We are just following the same scheme where the value of nil does not make much sense. (i.e. staging_disk_limit_mb)

/cc @ScarletTanager

@Amit-PivotalLabs
Copy link
Contributor

Sorry, I don't quite understand. Is there some functional reason why the default value from the spec needs to be duplicated in the template (which is something we strongly try to avoid), or is it just that the manifest looks odd?

@ghost
Copy link

ghost commented Jun 3, 2016

@Amit-PivotalLabs

It is just to make the generated manifest more readable. There is no technical reason that the values need to be duplicated. We are just being consistent in how these descriptive values are in both places.

@Amit-PivotalLabs
Copy link
Contributor

We strongly avoid duplicating default values in job specs and the manifest templates. If we are consistently doing this in other places in the manifest templates, we should remove those as well. How the manifest reads is less important at this time. Thanks for the PR, but will close it out for now.

@ScarletTanager ScarletTanager deleted the cfg_stagers branch June 8, 2016 14:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants