Version Logging - Only log on failure #1634
Conversation
| def to_docker_tag(version_str: str) -> str: | ||
| """Convert version string to docker tag""" | ||
| return version_str.replace("+", ".") | ||
|
|
||
|
|
||
| def get_metadata_version(package: str, version_str: str = "") -> str: | ||
| def version_resource() -> str: |
There was a problem hiding this comment.
This is an unhelpful name. Better might be get_best_version and probably a docstring like
"""returns the first version derived from tag, build, and pip"""
armory/utils/version.py
Outdated
| def to_docker_tag(version_str: str) -> str: | ||
| """Convert version string to docker tag""" | ||
| return version_str.replace("+", ".") | ||
|
|
||
|
|
||
| def get_metadata_version(package: str, version_str: str = "") -> str: | ||
| def version_resource() -> str: | ||
| checks = [get_tag_version, get_build_version, get_pip_version] |
There was a problem hiding this comment.
This is unduly complex, better might be
for check in checks:
version = check()
if version:
log.info(f"read {version} from {check.__name__}")
return version
else:
log.warning(f"could not get version from {check.__name__}
raise RuntimeError…
There was a problem hiding this comment.
the try/except is probably fine, but queuing up the failures is icky.
| raise RuntimeError("Unable to determine version number!") | ||
|
|
||
|
|
||
| def get_pip_version() -> str: |
There was a problem hiding this comment.
Better might be
def package_version_from_pip(package="armory-testbed"):
| ) | ||
|
|
||
|
|
||
| def get_build_version(version_str: str = "") -> str: |
There was a problem hiding this comment.
from x import "" is a syntax error
| def get_build_version(version_str: str = "") -> str: | ||
| """Retrieve the version from the build hook""" | ||
| try: | ||
| from armory.__about__ import __version__ as version_str |
There was a problem hiding this comment.
from armory.__about__ import __version__
return __version__
is clearer
| except ModuleNotFoundError: | ||
| log.warning("ERROR: Unable to extract version from __about__.py") | ||
| return version_str | ||
| if Path(project_root / ".git").is_dir(): |
There was a problem hiding this comment.
Subtle point here: scm.get_version could well succeed even if .git is not a directory. The failure is that scm failed, the presence of .git is just a proxy indicator of why. If you think this a likely error, you could test for .git presence after scm failure and add a hint to the log.
| package_name (str): The name of the package. | ||
| pretend_version (str): The version to pretend to be. | ||
| update_metadata (bool): Whether to update the metadata. | ||
|
|
||
| Example: | ||
| $ ARMORY_DEV_MODE=1 ARMORY_PRETEND_VERSION="1.2.3" armory --version | ||
| """ | ||
| old_version = get_metadata_version(package_name) | ||
| old_version = get_pip_version(package_name) |
There was a problem hiding this comment.
Currently get_pip_version takes no arguments. Pylance flagged this for me.
|
Validation checks (kinda hard to pytest missing directories and such). With In [2]: version_resource() With .git absent: I still need to check shallow clone and developer modes. |
|
This has been entirely scooped by #1635. Closing. |
No description provided.