feat(builder): make the builder pod timeout configurable

[aledbf]

closes #135

[aledbf]

@arschles please tell me the name to use for this 2 variables

[arschles]

@aledbf to complete #135, I'd like the timeout and tick durations to be specified as environment variables, instead of hard-coding them as constants here.

Regarding your question, the environment variables could be named OBJECT_STORAGE_WAIT_DURATION and OBJECT_STORAGE_TICK_DURATION.

Finally, on startup, the builder should check the values for those env vars to ensure valueOf(OBJECT_STORAGE_TICK_DURATION) < valueOf(OBJECT_STORAGE_WAIT_DURATION) (that is pseudocode) - otherwise, the git receive hook would always fail.

[aledbf]

instead of hard-coding them as constants here

I put that as default if there's no env variables defined. It's that ok?

[arschles]

Yes, that's ok if they're just defaults. In the code, though, I don't see the check for the environment variables.

Also, can you provide a separate set of tick and timeout variables for pod start and polling object storage. For example:

• BUILDER_POD_TICK_DURATION and BUILDER_POD_WAIT_DURATION for waitForPod call
• OBJECT_STORAGE_TICK_DURATION and OBJECT_STORAGE_WAIT_DURATION for the storage.ObjectExists calls

Also, if you put these values into the Config struct then you can specify the defaults there and the config package will take care of setting them if missing. Example code:

BuilderPodTickDurationMSec int `envconfig:"BUILDER_POD_TICK_DURATION" default:"100"`
BuilderPodWaitDurationMSec int `envconfig:"BUILDER_POD_WAIT_DURATION" default:"300000"` // 5 minutes
ObjectStorageTickDurationMSec int `envconfing:"OBJECT_STORAGE_TICK_DURATION" default:"500"`
ObjectStorageWaitDurationMSec int `envconfig:"OBJECT_STORAGE_WAIT_DURATION" default:"300000"` // 5 minutes

[aledbf]

ping @arschles

[arschles]

@aledbf other than my nitpick, LGTM

[smothiki]

Is this still in Progress ??

[arschles]

@smothiki good question - @aledbf I've labeled appropriately, since I'm assuming you're ready to go on this.

[aledbf]

@arschles yes, ready for review

[smothiki]

I was not reviewing it as it said IN progress.

[arschles]

@smothiki can you review now please?

[smothiki]

I was just asking as I gave an LGTM and the label says IN Progress. So just wondering If this is still in progress

[arschles]

@smothiki ok - any comments on the code?

[arschles]

@aledbf I took my LGTM out because I left a request for tests

[smothiki]

I don't have comments previously. But now you pointed at writing unit tests. Should I remove my LGTM or should the unit tests be a separate PR/issue

[arschles]

I'm ok with you leaving your LGTM on here. I'll review again when ready. Regarding unit tests, I'd prefer them to go into this PR (and, generally speaking, for all future builder PRs - even if it changes code that wasn't previously unit tested)

[smothiki]

+1

[aledbf]

@arschles tests added

[aledbf]

ping @arschles

[arschles]

@aledbf LGTM, thanks for adding tests. @smothiki can you review once more?