Skip to content

Do not override API parameters with default when calling initialize if they are already set #386

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
zippolyte merged 2 commits into from
Jun 4, 2019
Merged

Do not override API parameters with default when calling initialize if they are already set #386

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f7e71c7
Do not override API parameters with default when calling initialize i…
zippolyte Jun 4, 2019
ea5665e
Test precedence order
zippolyte Jun 4, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 5 additions & 6 deletions datadog/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,10 @@ def initialize(api_key=None, app_key=None, host_name=None, api_host=None,
:type mute: boolean
"""
# API configuration
api._api_key = api_key if api_key is not None else os.environ.get('DATADOG_API_KEY')
api._application_key = app_key if app_key is not None else os.environ.get('DATADOG_APP_KEY')
api._host_name = host_name if host_name is not None else get_hostname()
api._api_host = api_host if api_host is not None else \
os.environ.get('DATADOG_HOST', 'https://api.datadoghq.com')
api._api_key = api_key or api._api_key or os.environ.get('DATADOG_API_KEY')
api._application_key = app_key or api._application_key or os.environ.get('DATADOG_APP_KEY')
api._host_name = host_name or api._host_name or get_hostname()
api._api_host = api_host or api._api_host or os.environ.get('DATADOG_HOST', 'https://api.datadoghq.com')

# Statsd configuration
# ...overrides the default `statsd` instance attributes
Expand All @@ -90,5 +89,5 @@ def initialize(api_key=None, app_key=None, host_name=None, api_host=None,

# HTTP client and API options
for key, value in iteritems(kwargs):
attribute = "_{0}".format(key)
attribute = "_{}".format(key)
setattr(api, attribute, value)
136 changes: 65 additions & 71 deletions tests/unit/api/test_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import mock

# datadog
from datadog import initialize, api
from datadog import initialize, api, util
from datadog.api import (
Distribution,
Metric,
Expand Down Expand Up @@ -38,25 +38,6 @@
)


def preserve_environ_datadog(func):
"""
Decorator to preserve the original environment value.
"""
@wraps(func)
def wrapper(env_name, *args, **kwds):
environ_api_param = os.environ.get(env_name)
try:
return func(env_name, *args, **kwds)
finally:
# restore the original environ value
if environ_api_param:
os.environ[env_name] = environ_api_param
elif os.environ.get(env_name):
del os.environ[env_name]

return wrapper


class TestInitialization(DatadogAPINoInitialization):

def test_no_initialization_fails(self):
Expand Down Expand Up @@ -147,57 +128,70 @@ def test_initialize_options(self):
initialize(api_key=API_KEY, mute=False)
self.assertRaises(ApiError, MyCreatable.create)

def test_initialization_from_env(self):
"""
Set API parameters in `initialize` from environment variables.
"""
@preserve_environ_datadog
def test_api_params_from_env(env_name, attr_name, env_value):
"""
Set env_name environment variable to env_value
Assert api.attr_name = env_value
"""
os.environ[env_name] = env_value
initialize()
self.assertEqual(getattr(api, attr_name), env_value)

@preserve_environ_datadog
def test_api_params_default(env_name, attr_name, expected_value):
"""
Unset env_name environment variable
Assert api.attr_name = expected_value
"""
if os.environ.get(env_name):
del os.environ[env_name]
initialize()
self.assertEqual(getattr(api, attr_name), expected_value)

@preserve_environ_datadog
def test_api_params_from_params(env_name, parameter, attr_name, value):
"""
Unset env_name environment variable
Initialize API with parameter=value
Assert api.attr_name = value
"""
if os.environ.get(env_name):
del os.environ[env_name]
initialize(api_host='http://localhost')
self.assertEqual(api._api_host, 'http://localhost')

# Default values
test_api_params_default("DATADOG_API_KEY", "_api_key", None)
test_api_params_default("DATADOG_APP_KEY", "_application_key", None)
test_api_params_default("DATADOG_HOST", "_api_host", "https://api.datadoghq.com")

# From environment
test_api_params_from_env("DATADOG_API_KEY", "_api_key", env_value="apikey")
test_api_params_from_env("DATADOG_APP_KEY", "_application_key", env_value="appkey")
test_api_params_from_env("DATADOG_HOST", "_api_host", env_value="http://localhost")

# From parameters
test_api_params_from_params("DATADOG_API_KEY", "api_key", "_api_key", "apikey2")
test_api_params_from_params("DATADOG_APP_KEY", "app_key", "_application_key", "appkey2")
test_api_params_from_params("DATADOG_HOST", "api_host", "_api_host", "http://127.0.0.1")
def test_default_values(self):
initialize()

self.assertIsNone(api._api_key)
self.assertIsNone(api._application_key)
self.assertEqual(api._api_host, "https://api.datadoghq.com")
self.assertEqual(api._host_name, util.hostname.get_hostname())

def test_env_var_values(self):
os.environ["DATADOG_API_KEY"] = "API_KEY_ENV"
os.environ["DATADOG_APP_KEY"] = "APP_KEY_ENV"
os.environ["DATADOG_HOST"] = "HOST_ENV"

initialize()

self.assertEqual(api._api_key, "API_KEY_ENV")
self.assertEqual(api._application_key, "APP_KEY_ENV")
self.assertEqual(api._api_host, "HOST_ENV")
self.assertEqual(api._host_name, util.hostname.get_hostname())

del os.environ["DATADOG_API_KEY"]
del os.environ["DATADOG_APP_KEY"]
del os.environ["DATADOG_HOST"]

def test_function_param_value(self):
Copy link
Copy Markdown
Contributor

@nmuesch nmuesch Jun 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add a quick test that tests the order these are used? Like provided attribute >> existing attribute >> environment variable?

initialize(api_key="API_KEY", app_key="APP_KEY", api_host="HOST", host_name="HOSTNAME")

self.assertEqual(api._api_key, "API_KEY")
self.assertEqual(api._application_key, "APP_KEY")
self.assertEqual(api._api_host, "HOST")
self.assertEqual(api._host_name, "HOSTNAME")

def test_precedence(self):
# Initialize first with env vars
os.environ["DATADOG_API_KEY"] = "API_KEY_ENV"
os.environ["DATADOG_APP_KEY"] = "APP_KEY_ENV"
os.environ["DATADOG_HOST"] = "HOST_ENV"

initialize()

self.assertEqual(api._api_key, "API_KEY_ENV")
self.assertEqual(api._application_key, "APP_KEY_ENV")
self.assertEqual(api._api_host, "HOST_ENV")
self.assertEqual(api._host_name, util.hostname.get_hostname())

# Initialize again to check given parameters take precedence over already set value and env vars
initialize(api_key="API_KEY", app_key="APP_KEY", api_host="HOST", host_name="HOSTNAME")

self.assertEqual(api._api_key, "API_KEY")
self.assertEqual(api._application_key, "APP_KEY")
self.assertEqual(api._api_host, "HOST")
self.assertEqual(api._host_name, "HOSTNAME")

# Initialize again without specifying attributes to check that already initialized value takes precedence
initialize()

self.assertEqual(api._api_key, "API_KEY")
self.assertEqual(api._application_key, "APP_KEY")
self.assertEqual(api._api_host, "HOST")
self.assertEqual(api._host_name, "HOSTNAME")

del os.environ["DATADOG_API_KEY"]
del os.environ["DATADOG_APP_KEY"]
del os.environ["DATADOG_HOST"]


class TestResources(DatadogAPIWithInitialization):
Expand Down