Skip to content

Integration with results#7

Merged
Daniel-Amsellem merged 4 commits into
masterfrom
dev
Apr 12, 2020
Merged

Integration with results#7
Daniel-Amsellem merged 4 commits into
masterfrom
dev

Conversation

@Daniel-Amsellem

Copy link
Copy Markdown
Contributor

No description provided.

@Daniel-Amsellem Daniel-Amsellem requested a review from yaelpery April 1, 2020 08:18
@Daniel-Amsellem Daniel-Amsellem self-assigned this Apr 1, 2020

func (r *ResultsHTTPWrapper) GetByScanID(scanID string, limit, offset uint64) ([]ResultResponseModel, *ResultError, error) {
resp, err := getRequestWithLimitAndOffset(r.url+"/scan"+scanID+"/items", limit, offset)
resp, err := getRequestWithLimitAndOffset(r.url+"/"+scanID+"/items", limit, offset)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not use fmt.sprintf() ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. Done

Comment thread test/integration/result_test.go
viper.SetDefault(projectsPath, "projects")
viper.SetDefault(uploadsPath, "uploads")
viper.SetDefault(resultsPath, "results")
viper.SetDefault(resultsPath, "scan")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

results is better, we can mistake the scan for scans in the future.
I guess this is how the treafik sees this but consider changing this back

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope. it's just the current prefix before results API changes

if string(scan.Status) == string(requiredStatus) {
ch <- true
return
} else if string(scan.Status) == scansRESTApi.ScanFailed || string(scan.Status) == scansRESTApi.ScanCanceled {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice :) thanks!

@Daniel-Amsellem Daniel-Amsellem merged commit c8ff0f7 into master Apr 12, 2020
thtri referenced this pull request in thtri/ast-cli Apr 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants