Address pylint findings from CI (pylint 2.9)

Fixes ~30 lint issues surfaced by the CI job that were not caught locally
because the dev env runs a newer pylint. All pure lint fixes, no behavior
changes.

- osbenchmark/metrics.py:
  - W0108 unnecessary-lambda: use bare OsResultsStore constructor. The
    default's FileTestRunStore lambda is kept (with pragma) because the
    class is defined later in the file, so a forward reference would fail
    at import.
  - C0415 import-outside-toplevel: pragma on the deferred cloudwatch
    imports (deferral is intentional — imports at module top trigger a
    circular between metrics.py and the cloudwatch submodules).

- osbenchmark/metrics_stores/cloudwatch/metrics_store.py:
  - R1711 useless-return at end of to_externalizable.
  - W1309 pointless f-string in Insights query builder.

- osbenchmark/metrics_stores/cloudwatch/client.py:
  - C0415 pragma on deferred botocore.credentials imports (deferred to
    keep the assume-role branch cost-free when unused).

- tests/metrics_stores/cloudwatch/conftest.py:
  - E0702 raising-bad-type: bind fail_with to a local so type-narrowing
    is preserved.
  - Move botocore.exceptions import to the top of the module.

- tests/metrics_stores/cloudwatch/test_read_path.py,
  tests/metrics_stores/cloudwatch/test_write_path.py:
  - C0321 multi-statement lines expanded across ~10 sites in fake
    classes and one-line def bodies.
  - C0415/C0411 in-function conftest imports moved to module top and
    consolidated in first-party-then-local order.
  - W0611 unused imports removed.
  - test_write_path.py adds a module-level `# pylint: disable=protected-
    access` because the tests deliberately assert on `_buffered_events`
    and `_client_factory` to verify internal state (test-only, expected).

Signed-off-by: Michael Oviedo <mikeovi@amazon.com>

Author: OVI3D0

osbenchmark/metrics.py

@@ -380,7 +380,7 @@ def _datastore_factories(cfg):
 
 _DATASTORE_DEFAULT = {
     "metrics_store_class": None,  # populated below once classes are imported
-    "test_run_store": lambda cfg: FileTestRunStore(cfg),
+    "test_run_store": lambda cfg: FileTestRunStore(cfg),  # pylint: disable=unnecessary-lambda
     "results_store": lambda cfg: NoopResultsStore(),
     "test_run_store_log_message": "Creating file test_run store",
     "results_store_log_message": "Creating no-op results store",
@@ -1797,7 +1797,7 @@ def store_results(self, test_run):
     datastore_type="opensearch",
     metrics_store_class=OsMetricsStore,
     test_run_store=lambda cfg: CompositeTestRunStore(OsTestRunStore(cfg), FileTestRunStore(cfg)),
-    results_store=lambda cfg: OsResultsStore(cfg),
+    results_store=OsResultsStore,
     test_run_store_log_message="Creating OS test run store",
     results_store_log_message="Creating OS results store",
 )
@@ -1809,23 +1809,22 @@ def store_results(self, test_run):
 # module top, and they would re-enter this file mid-load if we imported
 # them eagerly here.
 def _cloudwatch_metrics_store_class():
+    # pylint: disable=import-outside-toplevel
     from osbenchmark.metrics_stores.cloudwatch.metrics_store import CloudWatchMetricsStore
     return CloudWatchMetricsStore
 
 
 def _cloudwatch_test_run_store(cfg):
+    # pylint: disable=import-outside-toplevel
     from osbenchmark.metrics_stores.cloudwatch.test_run_store import (
         CloudWatchTestRunStore, FileBackedCompositeTestRunStore,
     )
-    # Writes fan out to CloudWatch + file; reads come from file until commit
-    # #12 wires Logs Insights. The dedicated composite avoids regressing
-    # `osbenchmark compare` / `aggregate` / `list test-runs` while the
-    # CloudWatch read path is stubbed.
     return FileBackedCompositeTestRunStore(
         CloudWatchTestRunStore(cfg), FileTestRunStore(cfg))
 
 
 def _cloudwatch_results_store(cfg):
+    # pylint: disable=import-outside-toplevel
     from osbenchmark.metrics_stores.cloudwatch.results_store import CloudWatchResultsStore
     return CloudWatchResultsStore(cfg)
 

osbenchmark/metrics_stores/cloudwatch/client.py

@@ -107,6 +107,7 @@ def _assume_role(self, source_session: boto3.session.Session, role_arn: str) ->
         # credential resolver. The `_credentials` attribute is the documented
         # private-but-stable extension point — `create_assume_role_refresher`
         # itself sets credentials this way internally.
+        # pylint: disable=import-outside-toplevel
         from botocore.credentials import (
             DeferredRefreshableCredentials,
             create_assume_role_refresher,

osbenchmark/metrics_stores/cloudwatch/metrics_store.py

@@ -239,7 +239,6 @@ def to_externalizable(self, clear=False):
         # for interface parity but has no separate meaning here — flushing
         # is the only externalization step.
         self.flush(refresh=False)
-        return None
 
     # `close()` inherited from MetricsStore — its implementation already
     # calls self.flush(), clears meta-info, and sets opened=False, which
@@ -477,7 +476,7 @@ def get_percentiles(self, name, task=None, operation_type=None,
             else:
                 stats_parts.append(f"pct(`{safe_name}`, {p}) as `p_{_alias(p)}`")
         # count(*) so we can replicate OS's "no hits → None" behavior.
-        stats_parts.append(f"count(*) as count")
+        stats_parts.append("count(*) as count")
         query = (
             f"filter {filter_}\n"
             f"| stats " + ", ".join(stats_parts)

tests/metrics_stores/cloudwatch/conftest.py

@@ -28,6 +28,7 @@
 otherwise hit AWS. Each test gets a fresh ``FakeLogsClient`` so state
 between tests stays isolated.
 """
+import botocore.exceptions
 import pytest
 
 import osbenchmark.metrics_stores.cloudwatch.insights as insights_mod
@@ -94,7 +95,8 @@ def put_log_events(self, **kw):
         if self.fail_count > 0:
             self.fail_count -= 1
             if self.fail_with is not None:
-                raise self.fail_with
+                exc = self.fail_with
+                raise exc
         self.put_calls.append(kw)
 
     # ----- Logs Insights -----
@@ -161,6 +163,5 @@ def make_insights_rows(rows):
 
 
 def make_client_error(code, op="PutLogEvents"):
-    import botocore.exceptions
     return botocore.exceptions.ClientError(
         {"Error": {"Code": code, "Message": "test"}}, op)

tests/metrics_stores/cloudwatch/test_read_path.py

@@ -35,7 +35,6 @@
 
 from osbenchmark import exceptions
 from osbenchmark.metrics import SampleType
-from osbenchmark.metrics_stores.cloudwatch import insights
 from osbenchmark.metrics_stores.cloudwatch.insights import (
     InsightsQueryError,
     _flatten_rows,
@@ -50,7 +49,7 @@
     FileBackedCompositeTestRunStore,
 )
 
-from .conftest import make_insights_rows
+from .conftest import make_client_error, make_insights_rows
 
 
 # ----------------------------------------------------------------- Insights helper
@@ -177,8 +176,12 @@ def _make_factory(fake_client):
     class _F:
         def __init__(self, cw_cfg):
             self._client = fake_client
-        def probe_caller_identity(self): pass
-        def logs_client(self): return self._client
+
+        def probe_caller_identity(self):
+            pass
+
+        def logs_client(self):
+            return self._client
     return _F
 
 
@@ -362,7 +365,6 @@ def test_insights_error_degrades_to_empty(self, fake_logs_client, open_store):
         # logs:StartQuery. Reads should fail-soft to empty results so
         # the result-summary path doesn't crash the run. (Same fail-soft
         # contract as FileBackedCompositeTestRunStore.list.)
-        from .conftest import make_client_error
         def boom(**kw):
             raise make_client_error("AccessDeniedException", op="StartQuery")
         fake_logs_client.start_query = boom
@@ -470,13 +472,20 @@ class _FakeFile:
     def __init__(self):
         self.runs = {}
         self.stored = []
-    def store_test_run(self, run): self.stored.append(run)
+
+    def store_test_run(self, run):
+        self.stored.append(run)
+
     def find_by_test_run_id(self, tid):
         if tid in self.runs:
             return self.runs[tid]
         raise exceptions.NotFound("not local")
-    def list(self): return list(self.runs.values())
-    def store_html_results(self, run): pass
+
+    def list(self):
+        return list(self.runs.values())
+
+    def store_html_results(self, run):
+        pass
 
 
 class _FakeCW:
@@ -485,10 +494,14 @@ def __init__(self):
         self.list_called = False
         self.find_result = None
         self.find_called = False
-    def store_test_run(self, run): pass
+
+    def store_test_run(self, run):
+        pass
+
     def list(self):
         self.list_called = True
         return self.list_result
+
     def find_by_test_run_id(self, tid):
         self.find_called = True
         if self.find_result is not None:
@@ -497,7 +510,8 @@ def find_by_test_run_id(self, tid):
 
 
 class _Run:
-    def __init__(self, test_run_id): self.test_run_id = test_run_id
+    def __init__(self, test_run_id):
+        self.test_run_id = test_run_id
 
 
 class TestFileBackedComposite:
@@ -535,7 +549,8 @@ def test_list_merges_and_dedupes(self):
 
     def test_list_degrades_gracefully_on_cw_error(self):
         class _FailingCW:
-            def list(self): raise RuntimeError("boom (any exception)")
+            def list(self):
+                raise RuntimeError("boom (any exception)")
         f = _FakeFile()
         f.runs["a"] = _Run("a")
         c = FileBackedCompositeTestRunStore(_FailingCW(), f)
@@ -548,7 +563,9 @@ def test_write_fans_out(self):
         cw.stored = []
         # Add `stored` to _FakeCW for the test
         cw_stored = []
-        def store(run): cw_stored.append(run)
+
+        def store(run):
+            cw_stored.append(run)
         cw.store_test_run = store
         c = FileBackedCompositeTestRunStore(cw, f)
         c.store_test_run(_Run("x"))

tests/metrics_stores/cloudwatch/test_write_path.py

@@ -27,6 +27,7 @@
 EMF document builders, log-stream writer (chunking + retry), and the
 three store classes' write flows.
 """
+# pylint: disable=protected-access
 import datetime
 import json
 
@@ -36,7 +37,6 @@
 from osbenchmark import exceptions, metrics
 from osbenchmark.metrics_stores.cloudwatch import config as cw_config_mod
 from osbenchmark.metrics_stores.cloudwatch import emf
-from osbenchmark.metrics_stores.cloudwatch.config import CloudWatchConfig
 from osbenchmark.metrics_stores.cloudwatch.log_streams import (
     LogStreamWriter,
     _MAX_EVENTS_PER_BATCH,
@@ -52,7 +52,7 @@
     CloudWatchTestRunStore,
 )
 
-from .conftest import make_client_error
+from .conftest import _ResourceNotFound, make_client_error
 
 
 # --------------------------------------------------------------------- config
@@ -412,7 +412,6 @@ def test_auth_error_retried_once_then_bubbles(self, fake_logs_client):
             w.write_batch([{"timestamp": 1, "message": "x"}])
 
     def test_stream_recreated_on_resource_not_found(self, fake_logs_client):
-        from .conftest import _ResourceNotFound
         fake_logs_client.fail_with = _ResourceNotFound("stream gone")
         fake_logs_client.fail_count = 1
         w = LogStreamWriter(fake_logs_client, "g", "s")
@@ -596,7 +595,9 @@ def test_store_results_explodes_into_records(self, fake_logs_client, cw_config):
     def test_empty_results_does_not_provision(self, fake_logs_client, cw_config):
         class _Empty:
             test_run_id = "x"
-            def to_result_dicts(self): return []
+
+            def to_result_dicts(self):
+                return []
         store = CloudWatchResultsStore(
             cfg=_StoreCfg(),
             client_factory_class=_make_fake_factory(fake_logs_client),