Adding build numbers listing #3005
Conversation
k8s-ci-robot
added
cncf-cla: yes
|
/cc @mm4tt |
|
/assign @freehan |
test-utils/utils/bucket.go
Outdated
| q := url.Values{} | ||
| // GCS api doesn't like preceding '/', so remove it. | ||
| q.Set("prefix", strings.TrimPrefix(joinStringsAndInts(pathElements...)+"/", "/")) | ||
| q.Set("delimiter", "/") |
There was a problem hiding this comment.
This method is almost exactly ExpandListUrl, the only difference is this line.
Consider introducing a new method, e.g.
func (b *Bucket) buildUrl(parameters map[string]string) *url.URL {
q := url.Values{}
for key, val := range parameters {
q.Set(key, val)
}
return &url.URL{
Scheme: b.scheme,
...
}
}
And calling it from ExpandListUrl and ExpandListDirUrl
test-utils/utils/utils.go
Outdated
| @@ -218,7 +282,7 @@ func (u *Utils) CheckStartedStatus(job string, buildNumber int) (*StartedFile, e | |||
| defer response.Body.Close() | |||
| if response.StatusCode != http.StatusOK { | |||
| glog.Errorf("Got a non-success response %v while reading data for %v/%v/%v", response.StatusCode, job, buildNumber, "started.json") | |||
| return nil, fmt.Errorf("non-success response: %v", response.StatusCode) | |||
| return nil, err | |||
There was a problem hiding this comment.
Could you update the method documentation as well? This method doesn't return bool (second sentence).
test-utils/utils/utils.go
Outdated
| @@ -128,6 +129,35 @@ func (u *Utils) deref(job string, buildNumber int) (directory string, err error) | |||
| return out, nil | |||
| } | |||
|
|
|||
| // derefJob reads the file in GCS to figure out where the desired file is. We | |||
There was a problem hiding this comment.
I'm not sure if I follow this. Which file it reads? Looks like the method accepts a single string (job) and returns directory. Could you explain what it does in terms of 1) what is input and output 2) how input is transformed to output?
There was a problem hiding this comment.
I realized that this function had no sense. I removed it.
krzysied
force-pushed
the
bucket_dir_list
branch
from
9ad1553
to
2127338
Compare
|
/LGTM I'm not sure if I fully follow the dereferencing logic, but otherwise it looks good. |
krzysied
force-pushed
the
bucket_dir_list
branch
from
2127338
to
21010e3
Compare
k8s-ci-robot
added
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: freehan, krzysied, mm4tt 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 |
k8s-ci-robot
added
the
approved
Adding listing of available build numbersfor the given job from the Google project's GCS bucket.
ref kubernetes/perf-tests#469