Literal check on config section and key by mypy

[sakurai-youhei]

This PR will introduce a type check by mypy, which helps find illegal literals on the config section and key.

Relates #1646

[gbanasiak]

I like the idea of using mypy to make sure we covered all config sections/keys. At the same time we don't want to turn this into type hints introduction project for Rally. I don't think the effort is justified. I think we should aim at reducing the scope of this PR to absolute minimum. For instance modifications in esrally/client/synchronous.py and esrally/client/asynchronous.py are not relevant, so I think should be reverted.

I've played with this setup a bit and think that if we narrow down mypy to react to arg-type error only we should achieve the verification we want here, i.e. verification of arguments of config.Config class methods (see suggestion). If arg-type error is triggered for anything else than config.Config methods I would add # type: ignore[arg-type] annotation.

I've noticed that the list of literals is not complete due to lack of check_untyped_defs = true option for mypy.

[sakurai-youhei]

@gbanasiak Thank you for your comments. Please feel free to bring up anything.

• I agree to minimize the scope.
• I added # type: ignore[arg-type] with TODO comment.
• I didn't notice check_untyped_defs = true is needed. 👍
• I also added tests to check the literals to avoid human errors.

[gbanasiak]

Thank you for the next iteration. I really like the idea with unit tests :)

I did the following sanity check:

% ipython
[..]
In [1]: from esrally.config import Section, Key

In [2]: len(str(Section).split(","))
Out[2]: 21

In [3]: len(str(Key).split(","))
Out[3]: 105

Then in shell:

rg -oN --heading "g\.add\(config\.Scope\..*?, \".*?\", " | grep -Eo "\".*?\"" | sort -u > /tmp/section-tmp.txt
rg -oN --heading "g\.opts\(\".*?\"," | grep -Eo "\".*?\"" | sort -u >> /tmp/section-tmp.txt
sort -u /tmp/section-tmp.txt > /tmp/section.txt
wc -l /tmp/section.txt
rm /tmp/section-tmp.txt

rg -No --heading "g\.add\(config\.Scope\..*?, (\".*?\", ){2}" | awk '{print $3;}' | grep -Eo "\".*?\"" | sort -u > /tmp/key-tmp.txt
rg -oN --heading "g\.opts\(\".*?\)" | awk '{print $2;}' | grep -Eo "\".*?\"" | sort -u >> /tmp/key-tmp.txt
sort -u /tmp/key-tmp.txt > /tmp/key.txt
wc -l /tmp/key.txt
rm /tmp/key-tmp.txt

which produced:

      21 /tmp/section.txt
     115 /tmp/key.txt

Based on this I think we are right on target with the sections, but some of the keys are still missing. Just as an example (you can reproduce the above for complete list), the first missing key was distribution.dir which is present in esrally/mechanic/supplier.py.

Can you revisit the missing keys?

[sakurai-youhei]

Don't know why this line has been left until now, but black 23.3.0 reformats it now.

[sakurai-youhei]

@gbanasiak Here are the highlights in this round. I appreciate a fresh pair of eyes as this PR gets bigger. Thanks.

• Split out esrally.types to avoid circular references between esrally.config and other modules.
• Added one more test to check annotation for cfg and config around functions.
• The number of keys has reached 115, thanks to the test that discovers missing annotations.
• Replaced Union[T, None] with Optional[T].
• Eliminated # pylint: disable=C0301.

P.S. Although I wrote “fresh”, I just meant the second pair; not a new pair from someone else.

[sakurai-youhei]

git-rebased in the source branch because this PR gets aged.

[gbanasiak]

Apologies for delays. This looks good to me. Many thanks for this improvement.

The rebase had broken the diff in GH, but master...sakurai-youhei:literal-check-by-mypy allowed me to see the files that actually changed. I think it might be better to rebase only prior to opening a PR, and then merge master on top of the PR changes.

[sakurai-youhei]

@gbanasiak I didn't notice my git-rebase messed it up. I reverted it instead of appending something more.

Thank you very much for your review. I've learned a lot through this PR. I will merge this PR once all the CI checks have passed.