Skip to content

Commit 5be5d6b

Browse files
authored
refactor: get feature flags from features module (#1293)
<!-- Mandatory: Provide a clear, concise description of the changes and their purpose --> Get whether a feature flag is on or off as a constant from the new features module. It improves the code readability and slightly the performance as the flags are now cached. This work is to prepare the merging of the dispatcherd feature which is guarded by a flag.
1 parent dfa3d0f commit 5be5d6b

File tree

14 files changed

+203
-103
lines changed

14 files changed

+203
-103
lines changed

src/aap_eda/analytics/collector.py

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,14 @@
1616
from datetime import datetime
1717
from typing import Optional
1818

19+
from django.conf import settings
1920
from django.core.serializers.json import DjangoJSONEncoder
2021
from django.db import connection
21-
from flags.state import flag_enabled
2222
from insights_analytics_collector import Collector
2323

2424
from aap_eda.analytics import analytics_collectors, package, utils
2525
from aap_eda.conf.settings import application_settings
26+
from aap_eda.settings import features
2627

2728

2829
class AnalyticsCollector(Collector):
@@ -35,10 +36,7 @@ def _package_class() -> package.Package:
3536
return package.Package
3637

3738
def _is_shipping_configured(self) -> bool:
38-
if (
39-
not flag_enabled("FEATURE_EDA_ANALYTICS_ENABLED")
40-
or not utils.get_insights_tracking_state()
41-
):
39+
if not features.ANALYTICS or not utils.get_insights_tracking_state():
4240
self.logger.warning(
4341
"Insights for Event Driven Ansible is not enabled."
4442
)
@@ -91,8 +89,8 @@ def _save_last_gather(self) -> None:
9189
def gather(collection_type="scheduled", since=None, until=None, logger=None):
9290
if not logger:
9391
logger = logging.getLogger("aap_eda.analytics")
94-
if not flag_enabled("FEATURE_EDA_ANALYTICS_ENABLED"):
95-
logger.info("FEATURE_EDA_ANALYTICS_ENABLED is set to False.")
92+
if not features.ANALYTICS:
93+
logger.info(f"{settings.ANALYTICS_FEATURE_FLAG_NAME} is set to False.")
9694
return
9795
collector = AnalyticsCollector(
9896
collector_module=analytics_collectors,

src/aap_eda/core/management/commands/create_initial_data.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,13 @@
2323
from django.core.management import BaseCommand
2424
from django.db import transaction
2525
from django.db.models import Q
26-
from flags.state import flag_enabled
2726

2827
from aap_eda.conf import settings_registry
2928
from aap_eda.core import enums, models
3029
from aap_eda.core.models.utils import get_default_organization
3130
from aap_eda.core.tasking import enable_redis_prefix
3231
from aap_eda.core.utils.credentials import inputs_to_store
32+
from aap_eda.settings import features
3333

3434
NEW_HELP_TEXT = (
3535
"Red Hat Ansible Automation Platform base URL to authenticate with.",
@@ -1255,7 +1255,7 @@ def populate_credential_types(
12551255
for credential_type_data in credential_types:
12561256
# Analytics credential types are only available when it's enabled
12571257
if (
1258-
not flag_enabled("FEATURE_EDA_ANALYTICS_ENABLED")
1258+
not features.ANALYTICS
12591259
and credential_type_data.get("name")
12601260
in enums.SINGLETON_CREDENTIAL_TYPES
12611261
):

src/aap_eda/core/management/commands/gather_analytics.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,11 @@
1616
from datetime import timezone
1717

1818
from dateutil import parser
19+
from django.conf import settings
1920
from django.core.management.base import BaseCommand, CommandParser
20-
from flags.state import flag_enabled
2121

2222
from aap_eda.analytics import collector
23+
from aap_eda.settings import features
2324

2425

2526
class Command(BaseCommand):
@@ -71,8 +72,10 @@ def handle(self, *args, **options):
7172
opt_since = options.get("since")
7273
opt_until = options.get("until")
7374

74-
if not flag_enabled("FEATURE_EDA_ANALYTICS_ENABLED"):
75-
self.logger.error("FEATURE_EDA_ANALYTICS_ENABLED is set to False.")
75+
if not features.ANALYTICS:
76+
self.logger.error(
77+
f"{settings.ANALYTICS_FEATURE_FLAG_NAME} is set to False."
78+
)
7679
return
7780

7881
since = parser.parse(opt_since) if opt_since else None

src/aap_eda/core/management/commands/rqworker.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@
1717
from django.conf import settings
1818
from django.core.management.base import BaseCommand, CommandParser
1919
from django_rq.management.commands import rqworker
20-
from flags.state import flag_enabled
20+
21+
from aap_eda.settings import features
2122

2223

2324
class Command(BaseCommand):
@@ -33,7 +34,7 @@ def add_arguments(self, parser: CommandParser) -> None:
3334
return rqworker.Command.add_arguments(self, parser)
3435

3536
def handle(self, *args, **options) -> None:
36-
if flag_enabled(settings.DISPATCHERD_FEATURE_FLAG_NAME):
37+
if features.DISPATCHERD:
3738
self.stderr.write(
3839
self.style.ERROR(
3940
"DISPATCHERD feature not implemented yet. "

src/aap_eda/core/management/commands/scheduler.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,9 +77,9 @@
7777
import rq_scheduler
7878
from django.conf import settings
7979
from django_rq.management.commands import rqscheduler
80-
from flags.state import flag_enabled
8180

8281
from aap_eda.core import tasking
82+
from aap_eda.settings import features
8383
from aap_eda.utils.logging import startup_logging
8484

8585
logger = logging.getLogger(__name__)
@@ -166,7 +166,7 @@ class Command(rqscheduler.Command):
166166
help = "Runs RQ scheduler with configured jobs."
167167

168168
def handle(self, *args, **options) -> None:
169-
if flag_enabled(settings.DISPATCHERD_FEATURE_FLAG_NAME):
169+
if features.DISPATCHERD:
170170
self.stderr.write(
171171
self.style.ERROR(
172172
"This command is not supported when "

src/aap_eda/settings/core.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,10 @@
1717
# Defines feature flags, and their conditions.
1818
# See https://cfpb.github.io/django-flags/
1919
DISPATCHERD_FEATURE_FLAG_NAME = "FEATURE_DISPATCHERD_ENABLED"
20+
ANALYTICS_FEATURE_FLAG_NAME = "FEATURE_EDA_ANALYTICS_ENABLED"
2021

2122
FLAGS = {
22-
"FEATURE_EDA_ANALYTICS_ENABLED": [
23+
ANALYTICS_FEATURE_FLAG_NAME: [
2324
{
2425
"condition": "boolean",
2526
"value": False,

src/aap_eda/settings/features.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
# Copyright 2025 Red Hat, Inc.
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
from functools import cache
16+
from typing import Any, Dict
17+
18+
from django.conf import settings
19+
from flags.state import flag_enabled
20+
21+
# Type hints for module attributes
22+
DISPATCHERD: bool
23+
ANALYTICS: bool
24+
25+
# Mapping of feature names to their corresponding setting names
26+
_FEATURE_FLAGS: Dict[str, str] = {
27+
"DISPATCHERD": "DISPATCHERD_FEATURE_FLAG_NAME",
28+
"ANALYTICS": "ANALYTICS_FEATURE_FLAG_NAME",
29+
}
30+
31+
32+
@cache
33+
def _get_feature(name: str) -> bool:
34+
"""Get the current value of a feature flag."""
35+
return flag_enabled(name)
36+
37+
38+
def __getattr__(name: str) -> Any:
39+
"""Dynamically provide access to feature flags as module attributes."""
40+
try:
41+
setting_name = _FEATURE_FLAGS[name]
42+
return _get_feature(getattr(settings, setting_name))
43+
except KeyError:
44+
raise AttributeError(
45+
f"module {__name__} has no attribute {name}"
46+
) from None

src/aap_eda/tasks/analytics.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,10 @@
1616

1717
import django_rq
1818
from ansible_base.lib.utils.db import advisory_lock
19-
from flags.state import flag_enabled
2019

2120
from aap_eda.analytics import collector, utils
2221
from aap_eda.core import tasking
22+
from aap_eda.settings import features
2323

2424
logger = logging.getLogger(__name__)
2525

@@ -36,7 +36,7 @@
3636
def schedule_gather_analytics(
3737
queue_name: str = ANALYTICS_TASKS_QUEUE, cancel: bool = False
3838
) -> None:
39-
if not flag_enabled("FEATURE_EDA_ANALYTICS_ENABLED"):
39+
if not features.ANALYTICS:
4040
return
4141
interval = utils.get_analytics_interval()
4242
logger.info(f"Schedule analytics to run in {interval} seconds")

tests/integration/analytics/test_collector.py

Lines changed: 14 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818

1919
import pytest
2020
from django.db import connection
21-
from django.test import override_settings
2221

2322
from aap_eda.analytics.collector import AnalyticsCollector, gather
2423

@@ -42,12 +41,11 @@ def collector():
4241
return collector
4342

4443

45-
@patch("aap_eda.analytics.collector.flag_enabled")
44+
@patch("aap_eda.analytics.collector.features.ANALYTICS", True)
4645
@patch("aap_eda.analytics.collector.AnalyticsCollector")
47-
def test_gather_when_enabled(mock_collector_cls, mock_flag_enabled):
46+
def test_gather_when_enabled(mock_collector_cls):
4847
"""Test gather function when FEATURE_EDA_ANALYTICS_ENABLED
4948
is set to True"""
50-
mock_flag_enabled.return_value = True
5149
mock_collector_cls.return_value = MagicMock()
5250
mock_logger = MagicMock()
5351

@@ -58,7 +56,6 @@ def test_gather_when_enabled(mock_collector_cls, mock_flag_enabled):
5856
logger=mock_logger,
5957
)
6058

61-
mock_flag_enabled.assert_called_once_with("FEATURE_EDA_ANALYTICS_ENABLED")
6259
mock_collector_cls.assert_called_once_with(
6360
collector_module=ANY,
6461
collection_type="manual",
@@ -67,27 +64,19 @@ def test_gather_when_enabled(mock_collector_cls, mock_flag_enabled):
6764
assert result is not None
6865

6966

70-
@patch("aap_eda.analytics.collector.flag_enabled")
71-
def test_gather_when_disabled(mock_flag_enabled):
67+
@patch("aap_eda.analytics.collector.features.ANALYTICS", False)
68+
def test_gather_when_disabled():
7269
"""Test gather function when FEATURE_EDA_ANALYTICS_ENABLED
7370
is set to False"""
74-
mock_flag_enabled.return_value = False
7571
mock_logger = MagicMock()
76-
7772
result = gather(logger=mock_logger)
78-
79-
mock_logger.info.assert_called_once_with(
80-
"FEATURE_EDA_ANALYTICS_ENABLED is set to False."
81-
)
8273
assert result is None
8374

8475

8576
@pytest.mark.django_db
86-
@patch("aap_eda.analytics.collector.flag_enabled")
77+
@patch("aap_eda.analytics.collector.features.ANALYTICS", True)
8778
@patch("aap_eda.analytics.collector.AnalyticsCollector")
88-
def test_gather_uses_default_logger(mock_collector_cls, mock_flag_enabled):
89-
mock_flag_enabled.return_value = True
90-
79+
def test_gather_uses_default_logger(mock_collector_cls):
9180
with patch(
9281
"aap_eda.analytics.collector.logging.getLogger"
9382
) as mock_get_logger:
@@ -119,20 +108,15 @@ def test_shipping_disabled_logs_warning(
119108
with patch(
120109
"aap_eda.analytics.utils.get_insights_tracking_state",
121110
return_value=insights_tracking_state,
111+
), patch(
112+
"aap_eda.analytics.collector.features.ANALYTICS", feature_flag_state
122113
):
123-
with override_settings(
124-
FLAGS={
125-
"FEATURE_EDA_ANALYTICS_ENABLED": [
126-
("boolean", feature_flag_state)
127-
]
128-
}
129-
):
130-
assert collector._is_shipping_configured() is expected
131-
132-
if not expected:
133-
collector.logger.warning.assert_called_once_with(
134-
"Insights for Event Driven Ansible is not enabled."
135-
)
114+
assert collector._is_shipping_configured() is expected
115+
116+
if not expected:
117+
collector.logger.warning.assert_called_once_with(
118+
"Insights for Event Driven Ansible is not enabled."
119+
)
136120

137121

138122
@pytest.mark.django_db

tests/integration/analytics/test_gather_analytics.py

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
from unittest import mock
1818

1919
import pytest
20+
from django.conf import settings
2021
from django.core.management import call_command
2122
from django.test import override_settings
2223

@@ -35,9 +36,12 @@
3536
)
3637
@pytest.mark.django_db
3738
@override_settings(
38-
FLAGS={"FEATURE_EDA_ANALYTICS_ENABLED": [("boolean", True)]},
3939
AUTOMATION_AUTH_METHOD="user-pass",
4040
)
41+
@mock.patch(
42+
"aap_eda.core.management.commands.gather_analytics.features.ANALYTICS",
43+
True,
44+
)
4145
def test_gather_analytics_invalid_settings(
4246
analytics_settings,
4347
caplog_factory,
@@ -124,14 +128,15 @@ def test_gather_analytics_invalid_settings(
124128
],
125129
)
126130
@pytest.mark.django_db
127-
@override_settings(
128-
FLAGS={"FEATURE_EDA_ANALYTICS_ENABLED": [("boolean", True)]}
129-
)
130131
@mock.patch(
131132
"aap_eda.core.management.commands.gather_analytics.collector.gather"
132133
)
134+
@mock.patch(
135+
"aap_eda.core.management.commands.gather_analytics.features.ANALYTICS",
136+
True,
137+
)
133138
def test_gather_analytics_command(
134-
mock_gather, analytics_settings, caplog_factory, args, log_level, expected
139+
analytics_settings, caplog_factory, args, log_level, expected
135140
):
136141
with mock.patch(
137142
"aap_eda.analytics.collector.application_settings",
@@ -161,7 +166,7 @@ def test_gather_analytics_command(
161166
"feature_flag_state, expected",
162167
[
163168
(True, "Either --ship or --dry-run needs to be set."),
164-
(False, "FEATURE_EDA_ANALYTICS_ENABLED is set to False."),
169+
(False, f"{settings.ANALYTICS_FEATURE_FLAG_NAME} is set to False."),
165170
],
166171
)
167172
def test_gather_analytics_command_by_ff_state(
@@ -170,20 +175,15 @@ def test_gather_analytics_command_by_ff_state(
170175
with mock.patch(
171176
"aap_eda.analytics.collector.application_settings",
172177
new=analytics_settings,
178+
), mock.patch(
179+
"aap_eda.core.management.commands.gather_analytics.features.ANALYTICS",
180+
feature_flag_state,
173181
):
174182
analytics_settings.INSIGHTS_TRACKING_STATE = True
175183
out = StringIO()
176184
logger = logging.getLogger("aap_eda.analytics")
177185
eda_log = caplog_factory(logger)
178-
with override_settings(
179-
FLAGS={
180-
"FEATURE_EDA_ANALYTICS_ENABLED": [
181-
("boolean", feature_flag_state)
182-
]
183-
}
184-
):
185-
command = "gather_analytics"
186-
call_command(command, stdout=out)
186+
call_command("gather_analytics", stdout=out)
187187

188188
assert any(
189189
record.levelname == "ERROR" and record.message == expected

0 commit comments

Comments
 (0)