Remove runtime pkg_resources dependency in default and server_ingester#7057
Conversation
|
Added local validation results while workflow runs are pending maintainer approval for fork PR CI. Local command executed: Result: all 3 targets passed locally. Could a maintainer approve the pending workflow runs so required checks can execute on this PR? Thanks. |
tensorboard/BUILD
Outdated
| @@ -466,6 +464,11 @@ py_library(name = "expect_absl_testing_absltest_installed") | |||
| # `pip install setuptools`. | |||
| py_library(name = "expect_pkg_resources_installed") | |||
There was a problem hiding this comment.
Do you mind removing this one, please?
| def _iter_entry_points(group): | ||
| """Returns entry points for a given group across Python versions.""" | ||
| entry_points = metadata.entry_points() | ||
| if hasattr(entry_points, "select"): |
There was a problem hiding this comment.
Do you mind adding a comment clarifying why this check exists?
Maybe:
# In newer Python versions, `metadata.entry_points()` returns an `EntryPoints`
# object with a `select()` method.
# Before "selectable" entry points existed, it would return a dictionary.
tensorboard/data/server_ingester.py
Outdated
| self._path = path | ||
| self._version = ( | ||
| pkg_resources.parse_version(version) | ||
| packaging_version.parse(version) |
There was a problem hiding this comment.
You'll need to reformat these lines for the linter check to pass.
- Remove unused expect_pkg_resources_installed BUILD target - Add clarifying comment for entry_points() version compatibility check - Collapse ternary expression in server_ingester.py for readability - Fix pre-existing Black formatting issues in touched files
|
Thanks for the review! Pushed a follow-up commit addressing all three comments:
Also fixed a few pre-existing Black formatting issues in the touched files (extra blank lines, unnecessary tuple parentheses) to help the |
|
Hi, Is there any intention to produce a release (patch ?) following this PR by any chance ? Many thanks for producing a fix for this issue either case 👍🏻 |
|
Our team is working on producing release 2.21 right now, but trying to update the protobuf dependency to match the one used by TensorFlow 2.21 has been challenging, since other dependency compatibility issues arise. I'll ask the team to include this change in that release and/or produce a patch release afterwards. |
|
Thanks @arcra 👍🏻 |
4 participants
Motivation for features / changes
Fixes #7003.
pkg_resourcesis removed in newer setuptools, which can break TensorBoard at import/runtime. This change removes runtime reliance onpkg_resourcesin the two affected code paths.Technical description of changes
tensorboard/default.py:pkg_resources.iter_entry_points(...)importlib.metadata.entry_points(...)with compatibility handling for different Python return shapes.tensorboard/data/server_ingester.py:pkg_resources.parse_version(...)packaging.version.parse(...).tensorboard/default_test.pynow patches_iter_entry_pointsand usesload()-style fake entry points.tensorboard/version_test.pynow validates PEP 440 behavior usingpackaging.version.expect_packaging_installedwherepackagingis now required.Detailed steps to verify changes work correctly (as executed by you)
Executed locally in an isolated venv:
python -m py_compile tensorboard/default.py tensorboard/default_test.py tensorboard/data/server_ingester.py tensorboard/version_test.pyPYTHONPATH=. python tensorboard/version_test.py(passes)Attempted but environment-limited locally:
bazel test //tensorboard:version_test //tensorboard:default_test //tensorboard/data:server_ingester_test(bazel not available in local shell)default_test.pyandserver_ingester_test.pywithout Bazel-generated artifacts hit local environment/import constraints.Alternate designs / implementations considered (or N/A)