From 2784995c1f42b601f0c86ebff3ba1eef21687f26 Mon Sep 17 00:00:00 2001 From: Nicole Aragao Date: Wed, 23 Feb 2022 14:57:33 -0300 Subject: [PATCH 1/4] Disable a quoting rule from flake8 This specific rule is incompatible with Black code style, which we will implement in the near future. --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index f876b9cf5..776838e85 100644 --- a/Makefile +++ b/Makefile @@ -48,7 +48,7 @@ swagger-valid: node_modules/swagger-cli/swagger-cli.js validate docs/swagger.yml lint-flake8: - flake8 . --ignore D203,W504,W605 --exclude quipucords/api/migrations,docs,build,.vscode,client,venv,deploy,quipucords/local_gunicorn.conf.py + flake8 . --ignore D203,W504,W605,Q000 --exclude quipucords/api/migrations,docs,build,.vscode,client,venv,deploy,quipucords/local_gunicorn.conf.py lint-pylint: find . -name "*.py" -not -name "*0*.py" -not -path "./build/*" -not -path "./docs/*" -not -path "./.vscode/*" -not -path "./client/*" -not -path "./venv/*" -not -path "./deploy/*" -not -path "./quipucords/local_gunicorn.conf.py" | DJANGO_SETTINGS_MODULE=quipucords.settings xargs $(PYTHON) -m pylint --load-plugins=pylint_django --disable=duplicate-code,wrong-import-order,useless-import-alias,unnecessary-pass,too-many-lines,raise-missing-from From 012daaddf7ad498db4f3835f167aae39b79dfd17 Mon Sep 17 00:00:00 2001 From: Nicole Aragao Date: Wed, 23 Feb 2022 15:04:35 -0300 Subject: [PATCH 2/4] Creates FeatureFlag class Parse feature flags from environment variables using function get_env_flags_from_environ(), making sure only variables that follow the pattern are added or updated. Creates unit tests for the class and helper fuction. --- quipucords/quipucords/featureflag.py | 46 ++++++ quipucords/quipucords/tests_feature_flags.py | 162 +++++++++++++++++++ 2 files changed, 208 insertions(+) create mode 100644 quipucords/quipucords/featureflag.py create mode 100644 quipucords/quipucords/tests_feature_flags.py diff --git a/quipucords/quipucords/featureflag.py b/quipucords/quipucords/featureflag.py new file mode 100644 index 000000000..dac9dc842 --- /dev/null +++ b/quipucords/quipucords/featureflag.py @@ -0,0 +1,46 @@ +"""Filters feature flags from system env variables.""" + +import os + +DEFAULT_FEATURE_FLAGS_VALUES = { + "OVERALL_STATUS": False, +} + +VALID_VALUES_FOR_ENV_VARIABLES = ["0", "1"] + + +class FeatureFlag: + """Handle environment variables and its status.""" + + def __init__(self): + """Attributes and values are generated dynamically. + + Attributes keys-values are received in initial_data. + """ + initial_data = self.get_feature_flags_from_env() + for key, value in initial_data.items(): + setattr(self, key, value) + + def is_feature_active(self, feature_name): + """Return attribute value.""" + try: + return getattr(self, feature_name) + except AttributeError: + raise ValueError(f"{feature_name=} is not a valid input.") + + @classmethod + def get_feature_flags_from_env(cls): + """Filter feature flags from environment variables.""" + feature_flags = DEFAULT_FEATURE_FLAGS_VALUES.copy() + for key, value in os.environ.items(): + if key.upper().startswith("QPC_FEATURE_"): + feature_name = key.upper().replace("QPC_FEATURE_", "") + feature_value = value.strip() + if feature_value in VALID_VALUES_FOR_ENV_VARIABLES: + feature_flags[feature_name] = bool(int(feature_value)) + else: + raise ValueError( + f"'{feature_value}' from '{key}' can't be converted " + "to int, verify your environment variables." + ) + return feature_flags diff --git a/quipucords/quipucords/tests_feature_flags.py b/quipucords/quipucords/tests_feature_flags.py new file mode 100644 index 000000000..c8658ac0a --- /dev/null +++ b/quipucords/quipucords/tests_feature_flags.py @@ -0,0 +1,162 @@ +"""Test the FeatureFlag class and helper function.""" + +import os +from unittest import mock + +import pytest + +from .featureflag import FeatureFlag + + +@pytest.mark.parametrize( + "env_name,env_value,feature_name,feature_value", + ( + ("QPC_FEATURE_TEST", "0", "TEST", False), + ("QPC_FEATURE_TEST", "1", "TEST", True), + ), +) +def test_get_feature_flags_from_env( + env_name, env_value, feature_name, feature_value +): + """Tests if function retrieves new variables set in .env.""" + with mock.patch.dict(os.environ, {env_name: env_value}): + dict_with_test_flags = FeatureFlag.get_feature_flags_from_env() + assert feature_name in dict_with_test_flags + assert dict_with_test_flags[feature_name] is feature_value + + +def test_if_returns_default_value_if_another_env_set(): + """Tests if function also returns variables in default dict.""" + with mock.patch.dict(os.environ, ({"QPC_FEATURE_TEST": "0"})): + dict_with_test_flags = FeatureFlag.get_feature_flags_from_env() + assert "OVERALL_STATUS" in dict_with_test_flags + assert "TEST" in dict_with_test_flags + assert dict_with_test_flags["OVERALL_STATUS"] is False + assert dict_with_test_flags["TEST"] is False + + +@pytest.mark.parametrize( + "env_name,env_value,feature_name,feature_value", + ( + ("QPC_FEATURE_OVERALL_STATUS", "1", "OVERALL_STATUS", True), + ("QPC_FEATURE_TEST", "0", "TEST", False), + ), +) +def test_if_value_for_env_default_list_gets_updated( + env_name, env_value, feature_name, feature_value +): + """Tests if function updates env variable in default.""" + with mock.patch.dict(os.environ, ({env_name: env_value})): + dict_with_test_flags = FeatureFlag.get_feature_flags_from_env() + assert feature_name in dict_with_test_flags + assert dict_with_test_flags[feature_name] is feature_value + + +def test_when_value_cant_be_cast_to_int(): + """Tests if function only updates values if it can be cast to int.""" + with mock.patch.dict( + os.environ, ({"QPC_FEATURE_OVERALL_STATUS": "wrong"}) + ), pytest.raises(ValueError) as exc: + FeatureFlag.get_feature_flags_from_env() + assert ( + str(exc.value) == "'wrong' from 'QPC_FEATURE_OVERALL_STATUS'" + " can't be converted to int, verify your" + " environment variables." + ) + + +@pytest.mark.parametrize( + "env_name,env_value", + ( + ("QPC_FEATURE_TEST", "10"), + ("QPC_FEATURE_TEST1", "3"), + ("QPC_FEATURE_TEST2", "2000"), + ), +) +def test_when_int_is_not_valid_value_for_env( + env_name, env_value, +): + """Test when int is not a valid value for env.""" + with mock.patch.dict(os.environ, ({env_name: env_value})), pytest.raises( + ValueError, match="can't be converted to int" + ): + FeatureFlag.get_feature_flags_from_env() + + +@pytest.mark.parametrize( + "env_name,env_value,feature_name", + ( + ("TEST_QPC_FEATURE_", "1", "TEST"), + ("TEST_QPC_FEATURE_", "1", "TEST_"), + ("QPC_TEST1_FEATURE_", "0", "TEST1"), + ("QPC_TEST1_FEATURE_", "0", "_TEST1"), + ("QPC_TEST1_FEATURE_", "0", "TEST1_"), + ), +) +def test_function_only_adds_names_follow_standard( + env_name, env_value, feature_name +): + """Tests if function only adds variables that start with QPC_FEATURE_.""" + with mock.patch.dict(os.environ, ({env_name: env_value})): + dict_with_test_flags = FeatureFlag.get_feature_flags_from_env() + assert feature_name not in dict_with_test_flags + + +@pytest.mark.parametrize( + "env_name,env_value,feature_name,feature_value", + ( + ("qpc_feature_test", "1", "TEST", True), + ("qpc_feature_TEST1", "0", "TEST1", False), + ("QPC_feature_TEST2", "0", "TEST2", False), + ("qpc_FEATURE_test3", "1", "TEST3", True), + ), +) +def test_if_function_is_not_case_sensitive( + env_name, env_value, feature_name, feature_value +): + """Tests if function is not case-sensitive.""" + with mock.patch.dict(os.environ, ({env_name: env_value})): + dict_with_test_flags = FeatureFlag.get_feature_flags_from_env() + assert feature_name in dict_with_test_flags + assert dict_with_test_flags[feature_name] is feature_value + + +@pytest.fixture +def setup_feature_flag_instance_for_tests(): + """Set up instance of FeatureFlag class for tests.""" + with mock.patch.dict(os.environ, {"QPC_FEATURE_TEST": "1"}): + feature_flag_instance = FeatureFlag() + return feature_flag_instance + + +def test_if_instance_contains_all_attributes( + setup_feature_flag_instance_for_tests, # pylint: disable=redefined-outer-name # noqa: E501 +): + """Tests if the constructor loads all attributes correctly.""" + assert hasattr(setup_feature_flag_instance_for_tests, "TEST") + assert hasattr(setup_feature_flag_instance_for_tests, "OVERALL_STATUS") + + +def test_if_instance_attributes_values_are_correct( + setup_feature_flag_instance_for_tests, # pylint: disable=redefined-outer-name # noqa: E501 +): + """Tests if the right values are attributed to attribute.""" + assert setup_feature_flag_instance_for_tests.TEST is True + assert setup_feature_flag_instance_for_tests.OVERALL_STATUS is False + + +def test_is_feature_active(setup_feature_flag_instance_for_tests): # pylint: disable=redefined-outer-name # noqa: E501 + """Tests method is_feature_active.""" + assert ( + setup_feature_flag_instance_for_tests.is_feature_active("TEST") is True + ) + assert ( + setup_feature_flag_instance_for_tests.is_feature_active( + "OVERALL_STATUS" + ) + is False + ) + with pytest.raises(ValueError): + setup_feature_flag_instance_for_tests.is_feature_active( + "FALSE_ATTRIBUTE" + ) From 77e97549b02dfef81a3d784d6b35d395c5748fa3 Mon Sep 17 00:00:00 2001 From: Nicole Aragao Date: Tue, 15 Feb 2022 15:52:20 -0300 Subject: [PATCH 3/4] Makes FeatureFlag globally accessible in Django --- quipucords/quipucords/settings.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/quipucords/quipucords/settings.py b/quipucords/quipucords/settings.py index c4c052d6d..8f264bde4 100644 --- a/quipucords/quipucords/settings.py +++ b/quipucords/quipucords/settings.py @@ -23,6 +23,8 @@ import logging import os +from .featureflag import FeatureFlag + # Get an instance of a logger logger = logging.getLogger(__name__) # pylint: disable=invalid-name @@ -405,3 +407,6 @@ def is_int(value): 'QPC_INSIGHTS_REPORT_SLICE_SIZE', '10000') if is_int(QPC_INSIGHTS_REPORT_SLICE_SIZE): QPC_INSIGHTS_REPORT_SLICE_SIZE = int(QPC_INSIGHTS_REPORT_SLICE_SIZE) + +# Load Feature Flags +QPC_FEATURE_FLAGS = FeatureFlag() From 57463c6c56b9a42a9ffb86370d933bd5acfead00 Mon Sep 17 00:00:00 2001 From: Nicole Aragao Date: Wed, 23 Feb 2022 14:44:41 -0300 Subject: [PATCH 4/4] Ignores asdf file --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index 9e61a2243..65f77c8ce 100644 --- a/.gitignore +++ b/.gitignore @@ -125,3 +125,6 @@ yarn.lock # symbolic link to roles quipucords/roles/ + +# asdf +.tool-versions