Skip to content
This repository was archived by the owner on Jun 26, 2025. It is now read-only.

Conversation

@salmanmashayekh
Copy link
Contributor

@salmanmashayekh salmanmashayekh commented Apr 22, 2025

Removing the deprecated distutils from the repo.

Replaced usage of distutils.util.strtobool with a custom str_to_bool function in audit.py and health.py for improved clarity and consistency. Updated landing.py to utilize importlib.metadata for package metadata retrieval.
@salmanmashayekh salmanmashayekh changed the title Refactor boolean conversion in audit and health conventions Remove deprecated distutils Apr 22, 2025
except DistributionNotFound:
package_metadata = metadata(graph.metadata.name)
return package_metadata
except PackageNotFoundError:
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice @salmanmashayekh looks like almost 1-to-1 replacement

def str_to_bool(value):
"""
Convert a string value to boolean.
Similar to distutils.util.strtobool but returns a boolean instead of an int.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a little worried about the similar to .. but returns a boolean instead of an int - mostly because microcosm-flask is used everywhere and this could have some side-effects that will be tricky to debug / will only show up in a while from now when someone pulls in latest microcosm-flask.

So if you want to go with this I would test thoroughly with at least 1-2 services that it works as expected

Copy link
Contributor

@adamhadani adamhadani left a comment

Choose a reason for hiding this comment

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

LGTM but with important comment / caveat on the str_to_bool function and backward compatibility.

@salmanmashayekh
Copy link
Contributor Author

updated the function to match the return type @adamhadani

Copy link
Contributor

@adamhadani adamhadani left a comment

Choose a reason for hiding this comment

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

LGTM, I would try this out locally with 1-2 services that use it, e.g. by pip install -e . this package into the venv of a service and run it locally

from collections.abc import Mapping
from functools import lru_cache
from pkg_resources import iter_entry_points
from importlib.metadata import entry_points
Copy link
Contributor

Choose a reason for hiding this comment

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

now using importlib instead of pkg_resources, this was last remaining references to this now deprecated package.

float_to_top = True
include_trailing_comma = True
known_first_party = microcosm_flask
extra_standard_library = pkg_resources
Copy link
Contributor

Choose a reason for hiding this comment

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

no longer needed now that we switched to importlib

"marshmallow>=3.0.0",
"microcosm>=4.0.0",
"microcosm-logging>=2.0.0",
"microcosm-logging>=2.1.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

once this is merged and released this version will be available - should fix remaining test-failures in CI for this PR cc/ @salmanmashayekh @lfmoisa-globality

"flake8-logging-format>=1.0.0",
"flake8-isort",
"flake8-logging-format>=1.0.0",
"flake8-print",
Copy link
Contributor

Choose a reason for hiding this comment

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

just sorting deps

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants