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

feat: Read plugin version from distribution, with fallback to package #1254

Closed
wants to merge 12 commits into from
32 changes: 31 additions & 1 deletion singer_sdk/helpers/_compat.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
"""Compatibility helpers."""
from __future__ import annotations

import pathlib

try:
from typing import final
Expand All @@ -12,4 +15,31 @@
# Running on pre-3.8 Python; use importlib-metadata package
import importlib_metadata as metadata # type: ignore

__all__ = ["metadata", "final"]

__all__ = ["metadata", "get_project_distribution", "final"]


# Future: replace with `importlib.metadata.packages_distributions()` introduced in 3.10
def get_project_distribution(file_path=None) -> metadata.Distribution | None:
"""Get project distribution.

This walks each distribution on `sys.path` looking for one whose installed paths
include the given path.

Args:
file_path: File path to find distribution for.

Returns:
A discovered Distribution or None.
"""
for dist in metadata.distributions():
try:
relative = pathlib.Path(file_path or __file__).relative_to(
dist.locate_file("")
)
except ValueError:
pass
else:
if dist.files and relative in dist.files:
return dist
return None
17 changes: 12 additions & 5 deletions singer_sdk/plugin_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import json
import logging
import os
import sys
from collections import OrderedDict
from pathlib import PurePath
from types import MappingProxyType
Expand All @@ -27,7 +28,7 @@
from singer_sdk.configuration._dict_config import parse_environment_config
from singer_sdk.exceptions import ConfigValidationError
from singer_sdk.helpers._classproperty import classproperty
from singer_sdk.helpers._compat import metadata
from singer_sdk.helpers._compat import get_project_distribution, metadata
from singer_sdk.helpers._secrets import SecretString, is_common_secret_key
from singer_sdk.helpers._util import read_json_file
from singer_sdk.helpers.capabilities import (
Expand Down Expand Up @@ -162,10 +163,16 @@ def plugin_version(cls) -> str:
Returns:
The package version number.
"""
try:
version = metadata.version(cls.name)
except metadata.PackageNotFoundError:
version = "[could not be detected]"
# get the file location of the subclass
path = sys.modules[cls.__module__].__file__
distribution = get_project_distribution(path)
if distribution:
version = str(distribution.metadata["Version"])
else:
try:
version = metadata.version(cls.name)
except metadata.PackageNotFoundError:
version = "[could not be detected]"
return version

@classproperty
Expand Down
26 changes: 26 additions & 0 deletions tests/core/test_helpers.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import platform
import site
from pathlib import Path

import pytest

from singer_sdk.helpers._compat import get_project_distribution, metadata


def test_get_project_distribution():
"""Test `get_project_distribution()`.

click is a representative example of a distributed package from our dependency tree.
Any similar singer_sdk dependency could be used in stead.
"""
if platform.system() == "Windows":
pytest.xfail("Doesn't pass on windows.")
Comment on lines +16 to +17
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this mean the package name can't be retrieved in Windows?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It means the lookup via distribution will not work, yes. The fallback to metadata.version(cls.name) was the primary mechanism before this change (it relies on the library name being the same as the class name), though I don't know how well it worked for Windows in the first place 🤔

Given the hurdles we have to jump through to try and infer the plugin version via distribution or library metadata, I still think we are better off adding __version__ = "<version>" in __init__.py and importing that for --about. poetry-dynamic-versioning supports substitution from a 0.0.0 placeholder in any file (including __init__.py by default), and this approach is standard in other python packages 🤷‍♂️ This PR specifically addresses this ask from @aaronsteers 👍

Copy link
Collaborator

Choose a reason for hiding this comment

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

@kgpayne Yeah, I'd say it's probably not worth spending time trying to solve a problem that's only present when migrating existing taps and targets that are published to PyPI. Wdyt?


site_package_paths = site.getsitepackages()
site_package_path = next(
pth for pth in site_package_paths if Path(pth).parts[-1] == "site-packages"
)
singer_sdk_dependency_path = Path(site_package_path) / "click" / "__init__.py"
discovered_dst = get_project_distribution(singer_sdk_dependency_path)
assert discovered_dst
assert discovered_dst.name == "click"