oc new-app should not time out while using a proxy #16187
Conversation
openshift-ci-robot
added
the
size/M
pkg/generate/git/config.go
Outdated
| "time" | ||
| ) | ||
|
|
||
| func GetConfigValues(jsonPaths []string, useGlobalConfig bool, dir string, env []string) (map[string]string, error) { |
There was a problem hiding this comment.
This should be using the command abstraction for testing.
openshift-ci-robot
added
size/XS
coreydaley
changed the title
|
/retest |
coreydaley
changed the title
|
@openshift/devex ptal |
|
/retest |
1 similar comment
|
/retest |
|
/unassign @smarterclayton |
pkg/generate/app/sourcelookup.go
Outdated
| gitRepo := git.NewRepositoryWithEnv(env) | ||
| _, err = CloneAndCheckoutSources(gitRepo, url.StringNoFragment(), url.URL.Fragment, tempSrc, "") | ||
| glog.V(4).Infof("Checking if %v requires authentication", url.StringNoFragment()) | ||
| _, _, err = gitRepo.TimedListRemote(10*time.Second, url.StringNoFragment(), "") |
There was a problem hiding this comment.
so this will respect HTTP_PROXY/etc env variables, presumably. does it also respect proxy configuration supplied in a .gitconfig, assuming that's a thing that can be done?
There was a problem hiding this comment.
@bparees it won't and I don't know that we want to go through the trouble of understanding your git environment. There's a few ways you can specify the proxy... you can use env vars, a global http.proxy setting, or a per-domain setting like: http.[my-domain].proxy. I discussed this with @coreydaley yesterday and it didn't seem worth the work just to decide whether we should display a warning about possibly requiring secrets for your build config.
There was a problem hiding this comment.
We agreed that a better experience is simply to have it fail fast and if it didn't work, then the warning can be safely ignored.
There was a problem hiding this comment.
@csrwng fair enough, especially since we're timing this command out after 10s now, so it won't hang for a long time.
the oc new-app command should not time out while checking if the supplied source repository requires authentication.
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bparees, coreydaley The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
openshift-merge-robot
added
the
approved
|
/retest |
2 similar comments
|
/retest |
|
/retest |
|
Automatic merge from submit-queue |
the oc new-app command should not time out while
checking if the supplied source repository requires
authentication.
Bugzilla: link