Skip to content
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

Add initial documentation for the execute_test command #198

Merged
merged 6 commits into from
Jul 6, 2022

Conversation

travisbenedict
Copy link
Contributor

Signed-off-by: Travis Benedict benedtra@amazon.com

Description

This PR is a first step towards documenting the usage of the execute_test command. This command can take a large number of arguments so I've only added the most common use cases for now.

Issues Resolved

Testing

  • New functionality includes testing

No testing required


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Travis Benedict <benedtra@amazon.com>
@travisbenedict
Copy link
Contributor Author

@Naarcha-AWS would you mind looking this over?

docs/api/execute-test.md Outdated Show resolved Hide resolved
`bool-expression` | Specify an expression that evaluates to a boolean value. | No
### Arguments

**Note**: This is not an exhaustive list of arguments.
Copy link
Contributor

Choose a reason for hiding this comment

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

Any specific reason we're not able to make it exhaustive list?

Copy link
Contributor Author

@travisbenedict travisbenedict May 31, 2022

Choose a reason for hiding this comment

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

There's somewhere around 20 more arguments to document that deal with things like specific configurations for provisioning or reporting. I didn't think it was worth taking the time to add those here since they're used for very advanced use cases.

I can add them in but it will take a little while

Copy link
Contributor

Choose a reason for hiding this comment

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

@travisbenedict: Is there a place where users can find additional arguments? Ok with not documenting the majority of them, just need to provide another resource if possible.

Copy link
Contributor Author

@travisbenedict travisbenedict Jun 9, 2022

Choose a reason for hiding this comment

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

I can add a section to the bottom for "Advanced Arguments" that lists them all

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are already documented in the help text 🤦‍♂️ I'll update the table

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Decided to split the exhaustive list into a separate table to avoid burying the examples

Copy link
Contributor

@Naarcha-AWS Naarcha-AWS left a comment

Choose a reason for hiding this comment

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

Made a few suggestions, mostly to switch from passive to active voice.

docs/api/execute-test.md Outdated Show resolved Hide resolved
docs/api/execute-test.md Outdated Show resolved Hide resolved
`bool-expression` | Specify an expression that evaluates to a boolean value. | No
### Arguments

**Note**: This is not an exhaustive list of arguments.
Copy link
Contributor

Choose a reason for hiding this comment

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

@travisbenedict: Is there a place where users can find additional arguments? Ok with not documenting the majority of them, just need to provide another resource if possible.

docs/api/execute-test.md Outdated Show resolved Hide resolved
docs/api/execute-test.md Outdated Show resolved Hide resolved
docs/api/execute-test.md Outdated Show resolved Hide resolved
docs/api/execute-test.md Show resolved Hide resolved
docs/api/execute-test.md Outdated Show resolved Hide resolved
docs/api/execute-test.md Outdated Show resolved Hide resolved
docs/api/execute-test.md Outdated Show resolved Hide resolved
Signed-off-by: Travis Benedict <benedtra@amazon.com>
Signed-off-by: Travis Benedict <benedtra@amazon.com>
@travisbenedict travisbenedict force-pushed the execute_test branch 2 times, most recently from a72f3d6 to 8b0e9a0 Compare June 9, 2022 22:34
travisbenedict and others added 2 commits June 9, 2022 17:37
Co-authored-by: Naarcha-AWS <97990722+Naarcha-AWS@users.noreply.github.com>
Signed-off-by: Travis Benedict <benedtra@amazon.com>
Co-authored-by: Naarcha-AWS <97990722+Naarcha-AWS@users.noreply.github.com>
Signed-off-by: Travis Benedict <benedtra@amazon.com>
Argument | Description | Required
:--- | :--- |:---
`workload` | The dataset and operations that execute during a test. See [OpenSearch Benchmark Workloads repository](https://github.com/opensearch-project/opensearch-benchmark-workloads) for more details on workloads. | Yes
`workload-params` | Parameters defined within each workload that can be overwritten, defined in the README of each workload. You can find an example of the eventdata workload [here](https://github.com/opensearch-project/opensearch-benchmark-workloads/tree/main/eventdata#parameters). | No
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: "You can find an example of the parameters for the eventdata workload"

:--- | :--- |:---
`workload` | The dataset and operations that execute during a test. See [OpenSearch Benchmark Workloads repository](https://github.com/opensearch-project/opensearch-benchmark-workloads) for more details on workloads. | Yes
`workload-params` | Parameters defined within each workload that can be overwritten, defined in the README of each workload. You can find an example of the eventdata workload [here](https://github.com/opensearch-project/opensearch-benchmark-workloads/tree/main/eventdata#parameters). | No
`test_procedure` | Test Procedures define the sequence of operations and parameters for a specific workload. When there is `test_procedure` specified, Benchmark selects the default. Define test procedures within each workload directory. You can fine an example test procedure [here](https://github.com/opensearch-project/opensearch-benchmark-workloads/blob/main/eventdata/test_procedures/default.json). | No
Copy link
Collaborator

Choose a reason for hiding this comment

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

  1. Nit: "When there is test_procedure specified" --> "When test_procedure specified". Also regarding this sentence, I think it should be "When there is no test_procedure specified, Benchmark selects the default"?
  2. Spelling error: fine--> find in the last sentence

@IanHoang
Copy link
Collaborator

IanHoang commented Jul 6, 2022

Added a couple comments but otherwise looks good to me

Copy link
Collaborator

@IanHoang IanHoang left a comment

Choose a reason for hiding this comment

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

LGTM

Signed-off-by: Travis Benedict <benedtra@amazon.com>
@travisbenedict travisbenedict merged commit bc28461 into opensearch-project:main Jul 6, 2022
@travisbenedict travisbenedict deleted the execute_test branch July 6, 2022 16:54
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