-
Notifications
You must be signed in to change notification settings - Fork 0
Single influx #31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Single influx #31
Conversation
xiaoyzhu
left a comment
There was a problem hiding this 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/.... |
There was a problem hiding this comment.
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", |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 != "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if userId == ""?
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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())) |
There was a problem hiding this comment.
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
No description provided.