feat(aws): Move tags enablement environment variable to settings #1004
Conversation
ViBiOh
force-pushed
the
vibioh/refactor_tags_envvar
branch
from
d2f4c3d to
8a29c3c
Compare
Signed-off-by: Vincent Boutour <vincent.boutour@datadoghq.com>
ViBiOh
force-pushed
the
vibioh/refactor_tags_envvar
branch
from
8a29c3c to
1eb5050
Compare
| ) | ||
|
|
||
|
|
||
| def get_fetch_s3_tags(): |
There was a problem hiding this comment.
imo, we could just import the setting directly in the cache function an use it. not a strict opinion, but it will be consistent with the other settings consts that we import.
There was a problem hiding this comment.
It was my first attempt, but with mocking involved, I needed to reload settings, then lambda_cache (for example), but lambda_cache was already patched with mocks and tests were deeply broken.
Using a getter allows me to only reload settings, and the rest of components will dynamically benefit from it.
There was a problem hiding this comment.
yeah I added this comment regarding consistency in settings vars imports. Otherwise, it's not really changing anything.
Regarding the patching, I think it's a workaround here to do getters. However, I think we could use a code refactor for reworking patching.
We can check that later
|
|
||
| @patch("caching.base_tags_cache.send_forwarder_internal_metrics") | ||
| @patch("caching.lambda_cache.send_forwarder_internal_metrics") | ||
| @patch.dict(os.environ, {"DD_FETCH_LAMBDA_TAGS": "true"}) |
There was a problem hiding this comment.
we can use something like
with patch.dict(os.environ, {"MY_ENV_VAR": "test_value"}):
to have local env changes, I think it would also allow not to reload settings module
There was a problem hiding this comment.
It's used here for example but it doesn't prevent the use of reload. If I remove it, the test is broken.
There was a problem hiding this comment.
ok, at least the local env change part still holds if we're sharing / re-setting the same env vars amongst tests
What does this PR do?
Refactor the load of the environment variables related to tags enrichment.
Motivation
Reduce number of calls to
os.environand centralize the value into the settings.Testing Guidelines
Additional Notes
Types of changes
Check all that apply