Allow S2I behavior to be overriden by caller #310
Conversation
| config.Source = sourceURL | ||
| } | ||
|
|
||
| b.source = onBuildSourceHandler{ |
There was a problem hiding this comment.
Not sure what you mean - if no override is provided, we now create a default downloader and set it. b.source is only set once.
There was a problem hiding this comment.
Ah, you meant 61 in the old file. Fixed.
There was a problem hiding this comment.
i liked the old line 61 better than your new one. :)
There was a problem hiding this comment.
Fixed it for you - including saving you from a govet error for anonymous
field declarations
On Mon, Oct 19, 2015 at 5:48 PM, Ben Parees [email protected]
wrote:
In pkg/build/strategies/onbuild/onbuild.go
#310 (comment)
:s.SetScripts([]string{}, []string{api.Assemble, api.Run})
- downloader, sourceUrl, err := scm.DownloaderForSource(config.Source)
- if err != nil {
return nil, err
- downloader := overrides.Downloader
- if downloader == nil {
d, sourceURL, err := scm.DownloaderForSource(config.Source)if err != nil {return nil, err}downloader = dconfig.Source = sourceURL- }
- b.source = onBuildSourceHandler{
i liked the old line 61 better than your new one. :)
—
Reply to this email directly or view it on GitHub
https://github.com/openshift/source-to-image/pull/310/files#r42430517.
There was a problem hiding this comment.
thanks, now yours is better :)
|
One small nit, but I"d like to see tests for that please. |
|
Assuming this hasn"t changed much since I reviewed it in your origin PR I"m Otherwise it"s going to need to wait until Monday when I have laptop access But +1 to TCs. Ben Parees | OpenShift
|
smarterclayton
force-pushed
the
add_binary_build
branch
from
c0029f6 to
7fdb779
Compare
|
Why does STI execute take an api.Config that might be different from the api.Config passed to it on creation? |
smarterclayton
force-pushed
the
add_binary_build
branch
from
7fdb779 to
9fa8c8f
Compare
|
Tests added. |
smarterclayton
force-pushed
the
add_binary_build
branch
3 times, most recently
from
25df5f4 to
fe5fbf3
Compare
|
Ok, race averted. |
| } | ||
| config.Source = sourceUrl | ||
|
|
||
| b.source = onBuildSourceHandler{downloader, s, &ignore.DockerIgnorer{}} |
There was a problem hiding this comment.
it is duping this line isn"t it?
smarterclayton
force-pushed
the
add_binary_build
branch
from
c99699a to
9554076
Compare
|
squish and lgtm. |
smarterclayton
force-pushed
the
add_binary_build
branch
from
9554076 to
d31d44f
Compare
|
[merge] On Mon, Oct 19, 2015 at 5:55 PM, Ben Parees [email protected]
|
Origin needs to be able to download and assemble the source itself and then pass it to s2i. This commit adds changes to allow an Overrides object to be passed to the s2i initializer code to provide a custom Downloader. This also makes changes to allow better reuse of upstream s2i code and removes a write to disk for the intermediate tar (which can be streamed instead).
|
[merge] |
|
Evaluated for source to image merge up to d31d44f |
|
Source To Image Merge Results: SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_sti/196/) |
Merged by openshift-bot
Origin needs to be able to download and assemble the source
itself and then pass it to s2i. This commit adds changes to
allow an Overrides object to be passed to the s2i initializer
code to provide a custom Downloader.
This also makes changes to allow better reuse of upstream s2i
code and removes a write to disk for the intermediate tar (which
can be streamed instead).
@mfojtik @bparees review, this is part of binary build