Skip to content

Commit

Permalink
Remove the exec() and eval() uses.
Browse files Browse the repository at this point in the history
  • Loading branch information
@netsettler
netsettler committed Sep 29, 2023
1 parent 0388242 commit bd57fa5
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 28 deletions.
35 changes: 19 additions & 16 deletions dcicutils/license_utils.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -512,10 +512,10 @@ class LicenseChecker:
needs to have its dependencies checked.
ALLOWED is a list of license names as returned by the various license frameworks. Because they rely on different
underlying tools the exact format of the names that result might vary. (For this reason, there is a regular
expression capability for this particular attribute, so in addition to just a string, you can also use
underlying tools the exact format of the names that result might vary. For this reason, there is a regular
expression capability for this particular attribute. In addition to just a string, you can also use
{"pattern": "<regexp>"} For very long regular expressions, {"pattern": ["<regexp-part-1>", ...]} will
concatenate all the parts into a single regexp so they can be gracefully broken over lines in the .jsonc
concatenate all the parts into a single regexp, so they can be gracefully broken over lines in the .jsonc
source file. If regexp flags are requierd, use {"pattern" "<regexp>", "flags": ["flag1", ...]}.
EXPECTED_MISSING_LICENSES is a list of libraries that are expected to have no license information.
Expand Down Expand Up @@ -752,8 +752,7 @@ def lookup_checker(cls, checker_name: str, autoload: bool = False) -> Type[Licen
policy_dir = LicenseOptions.POLICY_DIR or POLICY_DIR
PRINT(f"Looking for custom policy {checker_name} in {policy_dir} ...")
result = find_or_create_license_class(policy_name=checker_name,
policy_dir=policy_dir,
for_env=globals())
policy_dir=policy_dir)
if result:
return result
raise InvalidParameterError(parameter='checker_name', value=checker_name,
Expand Down Expand Up @@ -872,7 +871,7 @@ def find_policy_data(policy_name: str, policy_dir: Optional[str] = None,
return data


def find_or_create_license_class(*, policy_name: str, policy_dir: str, for_env,
def find_or_create_license_class(*, policy_name: str, policy_dir: str,
# This next argument should never be passed explicitly by callers other than
# recursive calls to this function. -kmp 28-Sep-2023
_creation_attmpts_in_progress=None):
Expand All @@ -896,7 +895,7 @@ def find_or_create_license_class(*, policy_name: str, policy_dir: str, for_env,
license_frameworks = policy_data.get('LICENSE_FRAMEWORKS')
if license_frameworks == "ALL":
policy_data['LICENSE_FRAMEWORKS'] = LicenseFrameworkRegistry.all_framework_names()
parent_classes = [find_or_create_license_class(policy_name=parent_name, policy_dir=policy_dir, for_env=for_env,
parent_classes = [find_or_create_license_class(policy_name=parent_name, policy_dir=policy_dir,
_creation_attmpts_in_progress=_creation_attmpts_in_progress)
for parent_name in inherits_from]
defaulted_policy_data = default_policy_data(policy_name=policy_name, policy_data=policy_data,
Expand All @@ -913,13 +912,10 @@ def find_or_create_license_class(*, policy_name: str, policy_dir: str, for_env,
license_policy_class: Type[LicenseChecker] = new_class
decorator = LicenseCheckerRegistry.register_checker(name=policy_name)
registered_class = decorator(license_policy_class)
command = f"{license_checker_class_name} = LicenseCheckerRegistry.lookup_checker({repr(policy_name)})"
if LicenseOptions.DEBUG: # pragma: no cover - this doesn't have to work for production
PRINT(f"Executing: {command}")
exec(command, for_env)
if LicenseOptions.DEBUG: # pragma: no cover - this doesn't have to work for production
PRINT(f" {license_checker_class_name}.LICENSE_FRAMEWORKS"
f" = {eval(license_checker_class_name, for_env).LICENSE_FRAMEWORKS!r}")
found_class = LicenseCheckerRegistry.lookup_checker(policy_name)
PRINT(f"Registered checker class {policy_name!r}"
f" with license_frameworks {conjoined_list(found_class.LICENSE_FRAMEWORKS)}.")
_creation_attmpts_in_progress.remove(policy_name)
return registered_class

Expand Down Expand Up @@ -1014,14 +1010,21 @@ def default_policy_data(*, policy_name: str, policy_data: AnyJsonData, parent_cl
return result


def load_license_policies(for_env, policy_dir=None):
def load_license_policies(policy_dir=None):
for policy_name in built_in_policy_names():
find_or_create_license_class(policy_name=policy_name, policy_dir=policy_dir, for_env=for_env)
find_or_create_license_class(policy_name=policy_name, policy_dir=policy_dir)


# This will cause the definitions of classes to in the predefined set to be exported by this library
# in case they need to be imported elsewhere, for example to use in unit-testing. Those are things like
# * ParkLabCommonLicenseChecker, etc.
# * C4InfrastructureLicenseChecker, etc.
# See license_policies/*.jsonc for a full list.
load_license_policies(for_env=globals())
load_license_policies()

ParkLabCommonLicenseChecker = LicenseCheckerRegistry.lookup_checker('park-lab-common')
ParkLabCommonServerLicenseChecker = LicenseCheckerRegistry.lookup_checker('park-lab-common-server')
ParkLabPipelineLicenseChecker = LicenseCheckerRegistry.lookup_checker('park-lab-pipeline')
ParkLabGplPipelineLicenseChecker = LicenseCheckerRegistry.lookup_checker('park-lab-gpl-pipeline')
C4InfrastructureLicenseChecker = LicenseCheckerRegistry.lookup_checker('c4-infrastructure')
C4PythonInfrastructureLicenseChecker = LicenseCheckerRegistry.lookup_checker('c4-python-infrastructure')
16 changes: 4 additions & 12 deletions test/test_license_utils.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -959,31 +959,23 @@ class TestChecker(LicenseChecker):

# This tests the find part
test_registry['test'] = TestChecker
assert find_or_create_license_class(policy_name='test', policy_dir='ignored',
for_env='ignored') == TestChecker
assert find_or_create_license_class(policy_name='test', policy_dir='ignored') == TestChecker
mock_find_policy_data.assert_not_called()

mock_find_policy_data.return_value = {"inherits_from": []}
local_env = locals()
# The command that gets executed will expect to use LicenseCheckerRegistry, which would be in globals()
# but is not in locals(), so we have to add it. -kmp 29-Sep-2023
local_env["LicenseCheckerRegistry"] = license_utils_module.LicenseCheckerRegistry
policy_class = find_or_create_license_class(policy_name='something', policy_dir='/my/policy/dir',
for_env=local_env)
assert local_env["SomethingLicenseChecker"] == policy_class # check that it got installed in environment
policy_class = find_or_create_license_class(policy_name='something', policy_dir='/my/policy/dir')
assert issubclass(policy_class, LicenseChecker)


def test_load_license_policies():
test_policy_names = ['my_project', 'your_project']
policy_dir_for_testing = 'some/dir/'
some_env = 'some-env'
with mock.patch.object(license_utils_module, "find_or_create_license_class") as mock_find_or_create_license_class:
with mock.patch.object(license_utils_module, "built_in_policy_names") as mock_built_in_policy_names:
mock_built_in_policy_names.return_value = test_policy_names
load_license_policies(policy_dir=policy_dir_for_testing, for_env=some_env)
load_license_policies(policy_dir=policy_dir_for_testing)
mock_find_or_create_license_class.assert_has_calls([
mock.call(policy_name=policy_name, policy_dir=policy_dir_for_testing, for_env=some_env)
mock.call(policy_name=policy_name, policy_dir=policy_dir_for_testing)
for policy_name in test_policy_names
])

Expand Down

0 comments on commit bd57fa5

Please sign in to comment.