[
https://issues.apache.org/jira/browse/SPARK-60168?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Edward Lei updated SPARK-60168:
-------------------------------
Description:
Five `# type: ignore[union-attr]` comments in PySpark suppress mypy errors that
arise only because `SparkContext._jvm` and `SparkContext._gateway` are typed
`Optional` (they are `None` until a `SparkContext` is created), even though the
surrounding code requires a live JVM.
This follows SPARK-58346, which removed similar ignores by making the non-None
assumption explicit with an `assert`, the same idiom `SparkContext` itself uses
(for example `assert self._jvm is not None` in `core/context.py`).
Proposed change:
- `sql/connect/session.py`, `sql/streaming/listener.py` (two places),
`ml/connect/io_utils.py`: assert the JVM or gateway handle is not None right
before using it. In `StreamingQueryProgress.fromJObject` the assert is
evaluated eagerly rather than only for events with observed metrics; the method
is only called from JVM callbacks, so a live JVM is guaranteed.
- `ml/wrapper.py` (`JavaWrapper.__del__`): read
`SparkContext._active_spark_context` into a local variable and check `_gateway
is not None` in the existing condition. This method already catches and ignores
any `Exception`, so an assert would add nothing here.
No user-facing change. At the four assert sites a missing JVM handle raises
`AssertionError` instead of `AttributeError`, and those paths cannot be reached
without a live JVM anyway (`SparkContext._jvm` is never reset to `None` once a
context has been created). Verified with mypy over `python/pyspark` and the
existing `test_streaming_listener`, `test_wrapper` and
`test_legacy_mode_classification` tests.
was:
Five `# type: ignore[union-attr]` comments in PySpark suppress mypy errors that
arise only because `SparkContext._jvm` and `SparkContext._gateway` are typed
`Optional` (they are `None` until a `SparkContext` is created), even though the
surrounding code requires a live JVM.
This follows SPARK-58346, which removed similar ignores by making the non-None
assumption explicit with an `assert`, the same idiom `SparkContext` itself uses
(for example `assert self._jvm is not None` in `core/context.py`).
Proposed change:
- `sql/connect/session.py`, `sql/streaming/listener.py` (two places),
`ml/connect/io_utils.py`: assert the JVM or gateway handle is not None right
before using it.
- `ml/wrapper.py` (`JavaWrapper.__del__`): read
`SparkContext._active_spark_context` into a local variable and check `_gateway
is not None` in the existing condition. This method already swallows all
exceptions, so an assert would be pointless there.
No user-facing change. The only behavior difference is `AssertionError` instead
of `AttributeError` if the JVM handle is missing in the first group, which
already could not work in that case. Verified with mypy over `python/pyspark`
and the existing `test_wrapper` and `test_streaming_listener` tests.
> Remove type: ignore[union-attr] for the JVM and gateway handles
> ---------------------------------------------------------------
>
> Key: SPARK-60168
> URL: https://issues.apache.org/jira/browse/SPARK-60168
> Project: Spark
> Issue Type: Improvement
> Components: PySpark
> Affects Versions: 5.0.0
> Reporter: Edward Lei
> Priority: Minor
>
> Five `# type: ignore[union-attr]` comments in PySpark suppress mypy errors
> that arise only because `SparkContext._jvm` and `SparkContext._gateway` are
> typed `Optional` (they are `None` until a `SparkContext` is created), even
> though the surrounding code requires a live JVM.
> This follows SPARK-58346, which removed similar ignores by making the
> non-None assumption explicit with an `assert`, the same idiom `SparkContext`
> itself uses (for example `assert self._jvm is not None` in `core/context.py`).
> Proposed change:
> - `sql/connect/session.py`, `sql/streaming/listener.py` (two places),
> `ml/connect/io_utils.py`: assert the JVM or gateway handle is not None right
> before using it. In `StreamingQueryProgress.fromJObject` the assert is
> evaluated eagerly rather than only for events with observed metrics; the
> method is only called from JVM callbacks, so a live JVM is guaranteed.
> - `ml/wrapper.py` (`JavaWrapper.__del__`): read
> `SparkContext._active_spark_context` into a local variable and check
> `_gateway is not None` in the existing condition. This method already catches
> and ignores any `Exception`, so an assert would add nothing here.
> No user-facing change. At the four assert sites a missing JVM handle raises
> `AssertionError` instead of `AttributeError`, and those paths cannot be
> reached without a live JVM anyway (`SparkContext._jvm` is never reset to
> `None` once a context has been created). Verified with mypy over
> `python/pyspark` and the existing `test_streaming_listener`, `test_wrapper`
> and `test_legacy_mode_classification` tests.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
