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

Check for pointless statements #1039

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions .pylintrc
Original file line number Diff line number Diff line change
Expand Up @@ -177,8 +177,6 @@ disable=print-statement,
R0916,
R1707,
R1722,
W0104,
W0105,
W0106,
W0107,
W0201,
Expand Down
12 changes: 6 additions & 6 deletions esrally/mechanic/mechanic.py
Original file line number Diff line number Diff line change
Expand Up @@ -439,12 +439,6 @@ def on_all_nodes_stopped(self):

@thespian.actors.requireCapability('coordinator')
class Dispatcher(actor.RallyActor):
def __init__(self):
super().__init__()
self.start_sender = None
self.pending = None
self.remotes = None

"""This Actor receives a copy of the startmsg (with the computed hosts
attached) and creates a NodeMechanicActor on each targeted
remote host. It uses Thespian SystemRegistration to get
Expand All @@ -456,6 +450,12 @@ def __init__(self):
Dispatcher.
"""

def __init__(self):
super().__init__()
self.start_sender = None
self.pending = None
self.remotes = None

@actor.no_retry("mechanic dispatcher")
def receiveMsg_StartEngine(self, startmsg, sender):
self.start_sender = sender
Expand Down
6 changes: 1 addition & 5 deletions esrally/metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -238,10 +238,6 @@ class MetaInfoScope(Enum):
"""
Cluster level meta-information is valid for all nodes in the cluster (e.g. the benchmarked Elasticsearch version)
"""
# host = 2
"""
Host level meta-information is valid for all nodes on the same host (e.g. the OS name and version)
"""
node = 3
"""
Node level meta-information is valid for a single node (e.g. GC times)
Expand Down Expand Up @@ -412,11 +408,11 @@ def flush(self, refresh=True):
raise NotImplementedError("abstract method")

def close(self):
self.logger.info("Closing metrics store.")
"""
Closes the metric store. Note that it is mandatory to close the metrics store when it is no longer needed as it only persists
metrics on close (in order to avoid additional latency during the benchmark).
"""
self.logger.info("Closing metrics store.")
self.flush()
self.clear_meta_info()
self.opened = False
Expand Down
2 changes: 1 addition & 1 deletion esrally/utils/opts.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,13 +146,13 @@ def __init__(self, argvalue):

def parse_options(self):
def normalize_to_dict(arg):
from elasticsearch.client import _normalize_hosts
"""
Return parsed comma separated host string as dict with "default" key.
This is needed to support backwards compatible --target-hosts for single clusters that are not
defined as a json string or file.
"""

from elasticsearch.client import _normalize_hosts
Copy link
Contributor

Choose a reason for hiding this comment

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

now im curious, why is this here and not at the top?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is related to initialization of loggers. We noticed that especially the elasticsearch dependency is problematic in that respect. A long time ago we had all imports at the top but the elasticsearch loggers were initialized when a module was loaded that imported elasticsearch which happened before Rally had a chance to configure logging properly. Therefore we never import any elasticsearch module at the top in Rally.

return {TargetHosts.DEFAULT: _normalize_hosts(arg)}

self.parsed_options = to_dict(self.argvalue, default_parser=normalize_to_dict)
Expand Down
3 changes: 3 additions & 0 deletions tests/mechanic/supplier_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -857,6 +857,7 @@ def test_missing_url(self):
"release.cache": "true"
}, template_renderer=renderer)
with self.assertRaises(exceptions.SystemSetupError) as ctx:
# pylint: disable=pointless-statement
# noinspection PyStatementEffect
repo.download_url
self.assertEqual("Neither config key [miss.url] nor [jdk.unbundled.miss_url] is defined.", ctx.exception.args[0])
Expand All @@ -868,6 +869,7 @@ def test_missing_cache(self):
"runtime.jdk.bundled": "false"
}, template_renderer=renderer)
with self.assertRaises(exceptions.SystemSetupError) as ctx:
# pylint: disable=pointless-statement
# noinspection PyStatementEffect
repo.cache
self.assertEqual("Mandatory config key [release.cache] is undefined.", ctx.exception.args[0])
Expand All @@ -880,6 +882,7 @@ def test_invalid_cache_value(self):
"release.cache": "Invalid"
}, template_renderer=renderer)
with self.assertRaises(exceptions.SystemSetupError) as ctx:
# pylint: disable=pointless-statement
# noinspection PyStatementEffect
repo.cache
self.assertEqual("Value [Invalid] for config key [release.cache] is not a valid boolean value.", ctx.exception.args[0])
Expand Down