Skip to content

Commit

Permalink
core: allow legacy modules to be used in 'hpi module install' for bac…
Browse files Browse the repository at this point in the history
…kwards compatibility

but show warning

kinda hacky, but hopefully we will simplify it further when we have more such legacy modules
  • Loading branch information
karlicoss committed Jun 7, 2022
1 parent dbd15a7 commit 119b295
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 4 deletions.
4 changes: 3 additions & 1 deletion misc/check_legacy_init_py.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
logger = logzero.logger


MSG = 'importing my.fbmessenger is DEPRECATED'
MSG = 'my.fbmessenger is DEPRECATED'

def expect(*cmd: str, should_warn: bool=True) -> None:
res = run(cmd, stderr=PIPE)
Expand Down Expand Up @@ -61,6 +61,7 @@ def check_ok(*cmd: str, **kwargs) -> None:
check_warn('-c', 'from my.fbmessenger import messages, dump_chat_history')
check_warn('-m', 'my.core', 'query' , 'my.fbmessenger.messages', '-o', 'pprint', '--limit=10')
check_warn('-m', 'my.core', 'doctor', 'my.fbmessenger')
check_warn('-m', 'my.core', 'module', 'requires', 'my.fbmessenger')

# todo kinda annoying it doesn't work when executed as -c (but does as script!)
# presumably because doesn't have proper line number information?
Expand All @@ -71,6 +72,7 @@ def check_ok(*cmd: str, **kwargs) -> None:
check_ok ('-c', 'from my.fbmessenger.export import messages, dump_chat_history')
check_ok ('-m', 'my.core', 'query' , 'my.fbmessenger.export.messages', '-o', 'pprint', '--limit=10')
check_ok ('-m', 'my.core', 'doctor', 'my.fbmessenger.export')
check_ok ('-m', 'my.core', 'module', 'requires', 'my.fbmessenger.export')

# NOTE:
# to check that overlays work, run something like
Expand Down
3 changes: 3 additions & 0 deletions my/core/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,9 @@ def _requires(modules: Sequence[str]) -> Sequence[str]:
mods = [module_by_name(module) for module in modules]
res = []
for mod in mods:
if mod.legacy is not None:
warning(mod.legacy)

reqs = mod.requires
if reqs is None:
error(f"Module {mod.name} has no REQUIRES specification")
Expand Down
30 changes: 27 additions & 3 deletions my/core/discovery_pure.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ class HPIModule(NamedTuple):
doc: Optional[str] = None
file: Optional[Path] = None
requires: Requires = None
legacy: Optional[str] = None # contains reason/deprecation warning


def ignored(m: str) -> bool:
Expand Down Expand Up @@ -75,9 +76,19 @@ def _is_not_module_src(src: Path) -> bool:


def _is_not_module_ast(a: ast.Module) -> bool:
marker = NOT_HPI_MODULE_VAR
return any(
getattr(node, 'name', None) == NOT_HPI_MODULE_VAR # direct definition
or any(getattr(n, 'name', None) == NOT_HPI_MODULE_VAR for n in getattr(node, 'names', [])) # import from
getattr(node, 'name', None) == marker # direct definition
or any(getattr(n, 'name', None) == marker for n in getattr(node, 'names', [])) # import from
for node in a.body
)


def _is_legacy_module(a: ast.Module) -> bool:
marker = 'handle_legacy_import'
return any(
getattr(node, 'name', None) == marker # direct definition
or any(getattr(n, 'name', None) == marker for n in getattr(node, 'names', [])) # import from
for node in a.body
)

Expand Down Expand Up @@ -156,7 +167,11 @@ def _modules_under_root(my_root: Path) -> Iterable[HPIModule]:
if ignored(m):
continue
a: ast.Module = ast.parse(f.read_text())
if _is_not_module_ast(a):

# legacy modules are 'forced' to be modules so 'hpi module install' still works for older modules
# a bit messy, will think how to fix it properly later
legacy_module = _is_legacy_module(a)
if _is_not_module_ast(a) and not legacy_module:
continue
doc = ast.get_docstring(a, clean=False)

Expand All @@ -166,12 +181,15 @@ def _modules_under_root(my_root: Path) -> Iterable[HPIModule]:
except Exception as e:
logging.exception(e)

legacy = f'{m} is DEPRECATED. Please refer to the module documentation.' if legacy_module else None

yield HPIModule(
name=m,
skip_reason=None,
doc=doc,
file=f.relative_to(my_root.parent),
requires=requires,
legacy=legacy,
)


Expand Down Expand Up @@ -209,6 +227,12 @@ def test_requires() -> None:
assert len(r) == 2 # fragile, but ok for now


def test_legacy_modules() -> None:
# shouldn't crash
module_by_name('my.reddit')
module_by_name('my.fbmessenger')


def test_pure() -> None:
"""
We want to keep this module clean of other HPI imports
Expand Down

0 comments on commit 119b295

Please sign in to comment.