Add clippy lint to CI / fix clippy lints

Author: Sushisource

.github/workflows/ci.yml

@@ -19,6 +19,7 @@ jobs:
             python: "3.12"
             docsTarget: true
             cloudTestTarget: true
+            clippyLinter: true
           - os: ubuntu-latest
             python: "3.8"
             protoCheckTarget: true
@@ -58,6 +59,8 @@ jobs:
       # https://github.com/python-poetry/poetry/pull/7694 are fixed
       - run: python -m pip install --upgrade wheel "poetry==1.3.2" poethepoet
       - run: poetry install --no-root --all-extras
+      - run: poe bridge-lint
+        if: ${{ matrix.clippyLinter }}
       - run: poe lint
       - run: poe build-develop
       - run: mkdir junit-xml

.gitignore

@@ -7,3 +7,4 @@ temporalio/bridge/temporal_sdk_bridge*
 /tests/helpers/golangserver/golangserver
 /tests/helpers/golangworker/golangworker
 /.idea
+/sdk-python.iml

pyproject.toml

@@ -81,6 +81,7 @@ lint = [
   {cmd = "pyright"},
   {ref = "lint-docs"},
 ]
+bridge-lint = { cmd = "cargo clippy -- -D warnings", cwd = "temporalio/bridge" }
 # TODO(cretz): Why does pydocstyle complain about @overload missing docs after
 # https://github.com/PyCQA/pydocstyle/pull/511?
 lint-docs = "pydocstyle --ignore-decorators=overload"

temporalio/bridge/src/client.rs

@@ -502,9 +502,9 @@ impl TryFrom<ClientConfig> for ClientOptions {
         if let Some(tls_config) = opts.tls_config {
             gateway_opts.tls_cfg(tls_config.try_into()?);
         }
-        return gateway_opts
+        gateway_opts
             .build()
-            .map_err(|err| PyValueError::new_err(format!("Invalid client config: {}", err)));
+            .map_err(|err| PyValueError::new_err(format!("Invalid client config: {}", err)))
     }
 }
 

temporalio/bridge/src/lib.rs

@@ -63,12 +63,12 @@ fn connect_client<'a>(
     runtime_ref: &runtime::RuntimeRef,
     config: client::ClientConfig,
 ) -> PyResult<&'a PyAny> {
-    client::connect_client(py, &runtime_ref, config)
+    client::connect_client(py, runtime_ref, config)
 }
 
 #[pyfunction]
 fn new_metric_meter(runtime_ref: &runtime::RuntimeRef) -> Option<metric::MetricMeterRef> {
-    metric::new_metric_meter(&runtime_ref)
+    metric::new_metric_meter(runtime_ref)
 }
 
 #[pyfunction]
@@ -77,7 +77,7 @@ fn init_runtime(telemetry_config: runtime::TelemetryConfig) -> PyResult<runtime:
 }
 
 #[pyfunction]
-fn raise_in_thread<'a>(py: Python<'a>, thread_id: std::os::raw::c_long, exc: &PyAny) -> bool {
+fn raise_in_thread(py: Python, thread_id: std::os::raw::c_long, exc: &PyAny) -> bool {
     runtime::raise_in_thread(py, thread_id, exc)
 }
 
@@ -87,7 +87,7 @@ fn start_dev_server<'a>(
     runtime_ref: &runtime::RuntimeRef,
     config: testing::DevServerConfig,
 ) -> PyResult<&'a PyAny> {
-    testing::start_dev_server(py, &runtime_ref, config)
+    testing::start_dev_server(py, runtime_ref, config)
 }
 
 #[pyfunction]
@@ -96,7 +96,7 @@ fn start_test_server<'a>(
     runtime_ref: &runtime::RuntimeRef,
     config: testing::TestServerConfig,
 ) -> PyResult<&'a PyAny> {
-    testing::start_test_server(py, &runtime_ref, config)
+    testing::start_test_server(py, runtime_ref, config)
 }
 
 #[pyfunction]
@@ -105,7 +105,7 @@ fn new_worker(
     client: &client::ClientRef,
     config: worker::WorkerConfig,
 ) -> PyResult<worker::WorkerRef> {
-    worker::new_worker(&runtime_ref, &client, config)
+    worker::new_worker(runtime_ref, client, config)
 }
 
 #[pyfunction]
@@ -114,5 +114,5 @@ fn new_replay_worker<'a>(
     runtime_ref: &runtime::RuntimeRef,
     config: worker::WorkerConfig,
 ) -> PyResult<&'a PyTuple> {
-    worker::new_replay_worker(py, &runtime_ref, config)
+    worker::new_replay_worker(py, runtime_ref, config)
 }

temporalio/bridge/src/metric.rs

@@ -221,9 +221,9 @@ fn build_metric_parameters(
 
 #[pymethods]
 impl MetricAttributesRef {
-    fn with_additional_attributes<'p>(
+    fn with_additional_attributes(
         &self,
-        py: Python<'p>,
+        py: Python,
         meter: &MetricMeterRef,
         new_attrs: HashMap<String, PyObject>,
     ) -> PyResult<Self> {
@@ -240,8 +240,8 @@ impl MetricAttributesRef {
     }
 }
 
-fn metric_key_value_from_py<'p>(
-    py: Python<'p>,
+fn metric_key_value_from_py(
+    py: Python,
     k: String,
     obj: PyObject,
 ) -> PyResult<metrics::MetricKeyValue> {
@@ -317,8 +317,8 @@ impl CustomMetricAttributes for BufferedMetricAttributes {
     }
 }
 
-pub fn convert_metric_events<'p>(
-    py: Python<'p>,
+pub fn convert_metric_events(
+    py: Python,
     events: Vec<MetricEvent<BufferedMetricRef>>,
     durations_as_seconds: bool,
 ) -> Vec<BufferedMetricUpdate> {
@@ -328,8 +328,8 @@ pub fn convert_metric_events<'p>(
         .collect()
 }
 
-fn convert_metric_event<'p>(
-    py: Python<'p>,
+fn convert_metric_event(
+    py: Python,
     event: MetricEvent<BufferedMetricRef>,
     durations_as_seconds: bool,
 ) -> Option<BufferedMetricUpdate> {

temporalio/bridge/src/runtime.rs

@@ -122,8 +122,7 @@ pub fn init_runtime(telemetry_config: TelemetryConfig) -> PyResult<RuntimeRef> {
         // Event loop is assumed to be running at this point
         let locals = pyo3_asyncio::TaskLocals::with_running_loop(py)?.copy_context(py)?;
         PyResult::Ok(locals)
-    })
-    .expect("Works");
+    })?;
 
     // Create core runtime which starts tokio multi-thread runtime
     let mut core = CoreRuntime::new(
@@ -136,11 +135,12 @@ pub fn init_runtime(telemetry_config: TelemetryConfig) -> PyResult<RuntimeRef> {
                 // Set task locals for each thread
                 Python::with_gil(|_| {
                     THREAD_TASK_LOCAL.with(|r| {
-                        std::cell::OnceCell::set(r, task_locals.clone()).expect("NOT ALREADY SET");
+                        std::cell::OnceCell::set(r, task_locals.clone())
+                            .expect("TaskLocals are only set once");
                     });
                     PyResult::Ok(())
                 })
-                .expect("Setting event loop works");
+                .expect("Setting per-thread python TaskLocals must work");
             }),
         },
     )
@@ -197,7 +197,7 @@ pub fn init_runtime(telemetry_config: TelemetryConfig) -> PyResult<RuntimeRef> {
     })
 }
 
-pub fn raise_in_thread<'a>(_py: Python<'a>, thread_id: std::os::raw::c_long, exc: &PyAny) -> bool {
+pub fn raise_in_thread(_py: Python, thread_id: std::os::raw::c_long, exc: &PyAny) -> bool {
     unsafe { pyo3::ffi::PyThreadState_SetAsyncExc(thread_id, exc.as_ptr()) == 1 }
 }
 
@@ -223,9 +223,9 @@ impl Drop for Runtime {
 
 #[pymethods]
 impl RuntimeRef {
-    fn retrieve_buffered_metrics<'p>(
+    fn retrieve_buffered_metrics(
         &self,
-        py: Python<'p>,
+        py: Python,
         durations_as_seconds: bool,
     ) -> Vec<BufferedMetricUpdate> {
         convert_metric_events(
@@ -316,7 +316,7 @@ impl TryFrom<MetricsConfig> for Arc<dyn CoreMeter> {
 
     fn try_from(conf: MetricsConfig) -> PyResult<Self> {
         if let Some(otel_conf) = conf.opentelemetry {
-            if !conf.prometheus.is_none() {
+            if conf.prometheus.is_some() {
                 return Err(PyValueError::new_err(
                     "Cannot have OpenTelemetry and Prometheus metrics",
                 ));
@@ -391,7 +391,7 @@ tokio::task_local! {
 
 thread_local! {
     pub(crate) static THREAD_TASK_LOCAL: std::cell::OnceCell<pyo3_asyncio::TaskLocals> =
-        std::cell::OnceCell::new();
+        const { std::cell::OnceCell::new() };
 }
 
 impl pyo3_asyncio::generic::Runtime for TokioRuntime {
@@ -421,9 +421,8 @@ impl pyo3_asyncio::generic::ContextExt for TokioRuntime {
     }
 
     fn get_task_locals() -> Option<pyo3_asyncio::TaskLocals> {
-        match TASK_LOCALS.try_with(|c| c.get().map(|locals| locals.clone())) {
-            Ok(locals) => locals,
-            Err(_) => None,
-        }
+        TASK_LOCALS
+            .try_with(|c| c.get().cloned())
+            .unwrap_or_default()
     }
 }

temporalio/bridge/worker.py

@@ -35,6 +35,9 @@
 import temporalio.bridge.temporal_sdk_bridge
 import temporalio.converter
 import temporalio.exceptions
+from temporalio.bridge.temporal_sdk_bridge import (
+    CustomSlotSupplier as BridgeCustomSlotSupplier,
+)
 from temporalio.bridge.temporal_sdk_bridge import PollShutdownError
 
 

temporalio/worker/_tuning.py

@@ -7,10 +7,10 @@
 
 from typing_extensions import TypeAlias
 
-import temporalio.bridge.temporal_sdk_bridge
 import temporalio.bridge.worker
 from temporalio.bridge.worker import (
     ActivitySlotInfo,
+    BridgeCustomSlotSupplier,
     CustomSlotSupplier,
     LocalActivitySlotInfo,
     SlotInfo,
@@ -160,9 +160,7 @@ def _to_bridge_slot_supplier(
             ),
         )
     elif isinstance(slot_supplier, CustomSlotSupplier):
-        return temporalio.bridge.temporal_sdk_bridge.CustomSlotSupplier(
-            _ErrorLoggingSlotSupplier(slot_supplier)
-        )
+        return BridgeCustomSlotSupplier(_ErrorLoggingSlotSupplier(slot_supplier))
     else:
         raise TypeError(f"Unknown slot supplier type: {slot_supplier}")
 

tests/worker/test_worker.py

@@ -509,6 +509,7 @@ def release_slot(self, ctx: SlotReleaseContext) -> None:
     ) as _w:
         await asyncio.sleep(1)
 
+
 def create_worker(
     client: Client,
     on_fatal_error: Optional[Callable[[BaseException], Awaitable[None]]] = None,