Closed srohr closed 2 years ago
thanks for the PR!
I can definitely see how excluding modules would be useful in practice and your implementation looks solid, but I'd prefer to keep these functions as-is
My hesitation is mostly with the interface of a list of stringified module names as an argument. It makes sense since we're trying to avoid importing those modules but I'm not sure it'd be obvious to users without tracking down the test case.
Since the function is the experimental module and generic (not coupled to alembic_utils) keeping it in a separate module feels like the right choice
Insofar as this function is useful in the first place (which i definitely think it is!) for finding alembic-utils objects (like views defined throughout a set of database models), the current implementation suffers from lack of flexibility.
If, for whatever reason you cannot import a module or package in the greater context of your root package. Say, like @srohr said, due to dev-deps or optional deps. or maybe side-effects, the mutation of the MetaData
as you import models, comes to mind). But whatever the reason, your only options are:
Truly, I think an explicit exclusions list is the most direct and least ambiguous way of solving the problem.
But an alternative that comes to mind is you might instead
try:
module = importlib.import_module(module_import_path)
except ImportError:
continue
else:
yield module
This seems less ideal to me, but is really the only other obvious solution to the problem.
This PR adds the ability to exclude specified import paths from calls to
collect_instances
,collect_subclasses
orwalk_modules
. This might be desirable if some module (or its submodules) contain imports of libraries you only have installed in a dev configuration, or if you have large/slow modules to walk.