Skip to content

Conversation

@cherplunk
Copy link

No description provided.

Copy link
Contributor

@xiaoyzhu xiaoyzhu left a comment

Choose a reason for hiding this comment

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

Please address the several questions in this review.Thx.

Workload profiler
------------------

HTTP client -> curl -XPOST localhost:7777/...../benchmarks/....
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add example API call + input json for both /benchmarks and /benchmark APIs? Right now it's not immediately clear how to use the two APIs.

client := &InfluxClient{
influxUrl: "localhost",
influxPort: "8086",
influxBackupUrl: "localhost",
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems odd. The InfluxClient struct definition in influx_client.go doesn't have a member called "InfluxBackupUrl". How would this work?

Copy link
Author

Choose a reason for hiding this comment

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

Ah. The influx client should not have been included in this request - I will fix that.

},
}

if userId != "" {
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens if userId == ""?

Copy link
Author

Choose a reason for hiding this comment

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

userId must have a non-empty value (and can be defined in the application json), otherwise the cluster will not be created and the request fails.


Intensity int
ServiceName string
BenchmarkAgentClient *clients.BenchmarkAgentClient
Copy link
Contributor

Choose a reason for hiding this comment

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

"BenchmarkAgentClient" is already contained in BaseBenchmarkRun...


if response.StatusCode() != 200 {
return false, errors.New("Unexpected response code: " + strconv.Itoa(response.StatusCode()))
log.Infof("Unexpected response code: " + strconv.Itoa(response.StatusCode()))
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't seem right, waiting for deployment ready we are expecting the status code to be 200 from the deployer

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.

4 participants