Use CLI-specific API endpoints so that fossa test works with push-only API keys
#253
Conversation
api/fossa/builds.go
Outdated
| q := url.Values{} | ||
| q.Add("locator", locator.String()) | ||
|
|
||
| // GetLatestBuild loads the most recent build for a revision. |
There was a problem hiding this comment.
What happens when no build exists? It would be great to add that in the comment.
There was a problem hiding this comment.
If the build doesn't exist then the API returns a 404, so this should return an error.
There was a problem hiding this comment.
Let's document this in the function comment.
cmd/fossa/cmd/test/test.go
Outdated
| if err != nil { | ||
| log.Fatalf("Could not marshal unresolved issues: %s", err) | ||
| } | ||
| fmt.Println(string(marshalled)) |
There was a problem hiding this comment.
I think we should keep this until we figure out a good way to pretty-print issues. I bet there are existing users that are doing something like:
fossa test | tail -n +1 | jq < something >
There was a problem hiding this comment.
I removed it because we're already printing the issues here, although I suppose not as JSON.
fossa-cli/cmd/fossa/cmd/test/test.go
Line 141 in d11b2c8
I'll work on a better solution for pretty-printing issues, but for the moment the data isn't returned by the API.
api/fossa/issues.go
Outdated
| @@ -17,15 +17,17 @@ type Issue struct { | |||
| Type string | |||
| } | |||
|
|
|||
| type Issues struct { | |||
There was a problem hiding this comment.
Why is this better than []Issue? Where possible, I would like to prefer simpler data structures.
There was a problem hiding this comment.
I'm debating whether we should return any issue data at all aside from the count if a push-only API key is used.
There was a problem hiding this comment.
Got it. Let's document this in the comment for Issues. Otherwise, this is very unintuitive.
cmd/fossa/cmd/test/test.go
Outdated
| case "FAILED": | ||
| return latestBuild, fmt.Errorf("failed to analyze build #%d: %s (visit FOSSA or contact support@fossa.io)", latestBuild.ID, latestBuild.Error) | ||
| return build, fmt.Errorf("failed to analyze build #%d: %s (visit FOSSA or contact support@fossa.io)", build.ID, build.Error) | ||
| default: |
There was a problem hiding this comment.
I would like to change this so that we continue polling on a specific build status (e.g. "IN PROGRESS"?) instead of polling by default. This way we can return an error by default (i.e. on unrecognised statuses) so we can detect bugs earlier.
There was a problem hiding this comment.
The set of possible statuses is "CREATED" | "ASSIGNED" | "RUNNING" | "SUCCEEDED" | "FAILED", and we'd want to keep polling on anything other than succeeded or failed.
There was a problem hiding this comment.
Yes, but one day that might change. When it does, I want an error to occur instead of infinite polling.
cmd/fossa/cmd/test/test.go
Outdated
| case "FAILED": | ||
| return latestBuild, fmt.Errorf("failed to analyze build #%d: %s (visit FOSSA or contact support@fossa.io)", latestBuild.ID, latestBuild.Error) | ||
| return build, fmt.Errorf("failed to analyze build #%d: %s (visit FOSSA or contact support@fossa.io)", build.ID, build.Error) | ||
| default: |
There was a problem hiding this comment.
Yes, but one day that might change. When it does, I want an error to occur instead of infinite polling.
api/fossa/builds.go
Outdated
| q := url.Values{} | ||
| q.Add("locator", locator.String()) | ||
|
|
||
| // GetLatestBuild loads the most recent build for a revision. |
There was a problem hiding this comment.
Let's document this in the function comment.
api/fossa/issues.go
Outdated
| @@ -17,15 +17,17 @@ type Issue struct { | |||
| Type string | |||
| } | |||
|
|
|||
| type Issues struct { | |||
There was a problem hiding this comment.
Got it. Let's document this in the comment for Issues. Otherwise, this is very unintuitive.
|
LGTM. Will wait until the API changes are in before merging. |
* Suuport --branch for container scans * Make sure branch flows through to API * Reformat merged files
Endpoints added in https://github.com/fossas/FOSSA/pull/2909