Improve docs in various ways #1332
Conversation
git 1.9 is 7 years old, way older than Python 3.8, and the pyenv issue was fixed.
This confused one user on Discuss: https://discuss.elastic.co/t/rally-could-not-connect-to-your-elasticsearch-metrics-store/283762/5.
pquentin
added
:Docs
| @@ -34,15 +34,6 @@ Rally uses automatic code formatters. They're enforced by ``make lint`` and you | |||
|
|
|||
| However, consider using editor integrations to do it automatically: you'll need to configure `black <https://black.readthedocs.io/en/stable/integrations/editors.html>`_ and `isort <https://github.com/PyCQA/isort/wiki/isort-Plugins>`_. | |||
|
|
|||
| Known Issues | |||
There was a problem hiding this comment.
Is there a specific reason we removed the known issue(s)?
There was a problem hiding this comment.
I mentioned it in the commit, the issue is fixed in pyenv
There was a problem hiding this comment.
Do'h 🤦 . Thanks for confirming :-)
There was a problem hiding this comment.
Reading that again, I see that my comment is terse and sounds like it was obvious. It was not, and GitHub's UI makes it difficult to see the commit descriptions. Sorry about that!
There was a problem hiding this comment.
Reading that again, I see that my comment is terse and sounds like it was obvious. It was not, and GitHub's UI makes it difficult to see the commit descriptions. Sorry about that!
Haha, not a problem. I did not assume any malice :- 01.02, /FORMAT ;-).
docs/metrics.rst
Outdated
| @@ -4,7 +4,7 @@ Metrics | |||
| Metrics Records | |||
| --------------- | |||
|
|
|||
| At the end of a race, Rally stores all metrics records in its metrics store, which is a dedicated Elasticsearch cluster. Rally stores the metrics in the indices ``rally-metrics-*``. It will create a new index for each month. | |||
| At the end of a race, Rally stores all metrics records in its metrics store, which is a dedicated Elasticsearch cluster, ie. not the cluster where Rally ran its benchmarks. Rally stores the metrics in the indices ``rally-metrics-*``. It will create a new index for each month. | |||
There was a problem hiding this comment.
Perhaps worth adding a link here to the configuration section on reporting/metrics (https://esrally.readthedocs.io/en/stable/configuration.html#reporting) ?
There was a problem hiding this comment.
I tried to use Sphinx's internal links, but failed to link to the subsection, so I linked directly to the online docs.
Please take another look!
pquentin
force-pushed
the
docs-proofread
branch
from
ebf003c to
90ee074
Compare
docs/metrics.rst
Outdated
| @@ -4,7 +4,7 @@ Metrics | |||
| Metrics Records | |||
| --------------- | |||
|
|
|||
| At the end of a race, Rally stores all metrics records in its metrics store, which is a dedicated Elasticsearch cluster. Rally stores the metrics in the indices ``rally-metrics-*``. It will create a new index for each month. | |||
| At the end of a race, Rally stores all metrics records in its metrics store, which is a dedicated Elasticsearch cluster, ie. not the cluster where Rally ran its benchmarks. Rally stores the metrics in the indices ``rally-metrics-*``. It will create a new index for each month. | |||
There was a problem hiding this comment.
Nit: we usually use i.e. in our documentation (see also https://www.merriam-webster.com/dictionary/i.e.).
There was a problem hiding this comment.
Thanks, I'll keep that in mind, but I've switched to a different wording
docs/telemetry.rst
Outdated
| @@ -31,6 +31,8 @@ You probably want to gain additional insights from a race. Therefore, we have ad | |||
| shard-stats Shard Stats Regularly samples nodes stats at shard level | |||
| data-stream-stats Data Streams Stats Regularly samples data streams stats | |||
|
|
|||
| .. warning:: | |||
There was a problem hiding this comment.
This should be a separate warning block, maybe before the output of the command line example. The sentence below ("Keep in mind that each telemetry device may incur a runtime overhead which can skew results.") is actually part of the command line output of esrally list telemetry and should not be removed.
There was a problem hiding this comment.
The alternative I see is duplicating the warning, please tell me if that's better
There was a problem hiding this comment.
Yes, I'd prefer a separate warning (above the example) directly in the docs. This fact is important enough that it warrants a warning in the docs and in the command line output IMHO.
There was a problem hiding this comment.
Sorry, I meant "I've updated the docs like you said, please take a look". I wasn't trying to ask you to repeat your argument a second time, hoping that would change your mind :D
Please review commit by commit, I can easily revert any of them.