Skip to content

Commit d69c746

Browse files
mambocabmambocab
committed
add profile mode contact point checking
1 parent 5516735 commit d69c746

File tree

3 files changed

+175
-22
lines changed

3 files changed

+175
-22
lines changed

‎CHANGELOG.rst‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@ Features
66
* Send keyspace in QUERY, PREPARE, and BATCH messages (PYTHON-678)
77
* Add IPv4Address/IPv6Address support for inet types (PYTHON-751)
88
* WriteType.CDC and VIEW missing (PYTHON-794)
9-
* Warn on Cluster init if contact points are specified but LBP isn't (PYTHON-812)
9+
* Warn on Cluster init if contact points are specified but LBP isn't (legacy mode) (PYTHON-812)
10+
* Warn on Cluster init if contact points are specified but LBP isn't (exection profile mode) (PYTHON-838)
1011
* Include hash of result set metadata in prepared stmt id (PYTHON-808)
1112

1213
Bug Fixes

‎cassandra/cluster.py‎

Lines changed: 70 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -242,10 +242,19 @@ class ExecutionProfile(object):
242242
Defaults to :class:`.NoSpeculativeExecutionPolicy` if not specified
243243
"""
244244

245-
def __init__(self, load_balancing_policy=None, retry_policy=None,
245+
# indicates if lbp was set explicitly or uses default values
246+
_load_balancing_policy_explicit = False
247+
248+
def __init__(self, load_balancing_policy=_NOT_SET, retry_policy=None,
246249
consistency_level=ConsistencyLevel.LOCAL_ONE, serial_consistency_level=None,
247250
request_timeout=10.0, row_factory=named_tuple_factory, speculative_execution_policy=None):
248-
self.load_balancing_policy = load_balancing_policy or default_lbp_factory()
251+
252+
if load_balancing_policy is _NOT_SET:
253+
self._load_balancing_policy_explicit = False
254+
self.load_balancing_policy = default_lbp_factory()
255+
else:
256+
self._load_balancing_policy_explicit = True
257+
self.load_balancing_policy = load_balancing_policy
249258
self.retry_policy = retry_policy or RetryPolicy()
250259
self.consistency_level = consistency_level
251260
self.serial_consistency_level = serial_consistency_level
@@ -259,6 +268,15 @@ class ProfileManager(object):
259268
def __init__(self):
260269
self.profiles = dict()
261270

271+
def _profiles_without_explicit_lbps(self):
272+
names = (profile_name for
273+
profile_name, profile in self.profiles.items()
274+
if not profile._load_balancing_policy_explicit)
275+
return tuple(
276+
'EXEC_PROFILE_DEFAULT' if n is EXEC_PROFILE_DEFAULT else n
277+
for n in names
278+
)
279+
262280
def distance(self, host):
263281
distances = set(p.load_balancing_policy.distance(host) for p in self.profiles.values())
264282
return HostDistance.LOCAL if HostDistance.LOCAL in distances else \
@@ -341,7 +359,14 @@ class Cluster(object):
341359
local_dc set (as is the default), the DC is chosen from an arbitrary
342360
host in contact_points. In this case, contact_points should contain
343361
only nodes from a single, local DC.
362+
363+
Note: In the next major version, if you specify contact points, you will
364+
also be required to also explicitly specify a load-balancing policy. This
365+
change will help prevent cases where users had hard-to-debug issues
366+
surrounding unintuitive default load-balancing policy behavior.
344367
"""
368+
# tracks if contact_points was set explicitly or with default values
369+
_contact_points_explicit = None
345370

346371
port = 9042
347372
"""
@@ -770,17 +795,12 @@ def __init__(self,
770795
771796
Any of the mutable Cluster attributes may be set as keyword arguments to the constructor.
772797
"""
773-
if contact_points is not _NOT_SET and load_balancing_policy is None:
774-
log.warn('Cluster.__init__ called with contact_points specified, '
775-
'but no load_balancing_policy. In the next major '
776-
'version, this will raise an error; please specify a '
777-
'load balancing policy. '
778-
'(contact_points = {cp}, lbp = {lbp}'
779-
''.format(cp=contact_points, lbp=load_balancing_policy))
780-
781798
if contact_points is not None:
782-
if contact_points is _UNSET_ARG:
799+
if contact_points is _NOT_SET:
800+
self._contact_points_explicit = False
783801
contact_points = ['127.0.0.1']
802+
else:
803+
self._contact_points_explicit = True
784804

785805
if isinstance(contact_points, six.string_types):
786806
raise TypeError("contact_points should not be a string, it should be a sequence (e.g. list) of strings")
@@ -853,11 +873,35 @@ def __init__(self,
853873
raise ValueError("Clusters constructed with execution_profiles should not specify legacy parameters "
854874
"load_balancing_policy or default_retry_policy. Configure this in a profile instead.")
855875
self._config_mode = _ConfigMode.LEGACY
876+
856877
else:
857878
if execution_profiles:
858879
self.profile_manager.profiles.update(execution_profiles)
859880
self._config_mode = _ConfigMode.PROFILES
860881

882+
if self._contact_points_explicit:
883+
if self._config_mode is _ConfigMode.PROFILES:
884+
default_lbp_profiles = self.profile_manager._profiles_without_explicit_lbps()
885+
if default_lbp_profiles:
886+
log.warn(
887+
'Cluster.__init__ called with contact_points '
888+
'specified, but load-balancing policies are not '
889+
'specified in some ExecutionProfiles. In the next '
890+
'major version, this will raise an error; please '
891+
'specify a load-balancing policy. '
892+
'(contact_points = {cp}, '
893+
'EPs without explicit LBPs = {eps})'
894+
''.format(cp=contact_points, eps=default_lbp_profiles))
895+
else:
896+
if load_balancing_policy is None:
897+
log.warn(
898+
'Cluster.__init__ called with contact_points '
899+
'specified, but no load_balancing_policy. In the next '
900+
'major version, this will raise an error; please '
901+
'specify a load-balancing policy. '
902+
'(contact_points = {cp}, lbp = {lbp})'
903+
''.format(cp=contact_points, lbp=load_balancing_policy))
904+
861905
self.metrics_enabled = metrics_enabled
862906
self.ssl_options = ssl_options
863907
self.sockopts = sockopts
@@ -999,6 +1043,21 @@ def add_execution_profile(self, name, profile, pool_wait_timeout=5):
9991043
raise ValueError("Cannot add execution profiles when legacy parameters are set explicitly.")
10001044
if name in self.profile_manager.profiles:
10011045
raise ValueError("Profile %s already exists")
1046+
contact_points_but_no_lbp = (
1047+
self._contact_points_explicit and not
1048+
profile._load_balancing_policy_explicit)
1049+
if contact_points_but_no_lbp:
1050+
log.warn(
1051+
'Tried to add an ExecutionProfile with name {name}. '
1052+
'{self} was explicitly configured with contact_points, but '
1053+
'{ep} was not explicitly configured with a '
1054+
'load_balancing_policy. In the next major version, trying to '
1055+
'add an ExecutionProfile without an explicitly configured LBP '
1056+
'to a cluster with explicitly configured contact_points will '
1057+
'raise an exception; please specify a load-balancing policy '
1058+
'in the ExecutionProfile.'
1059+
''.format(name=repr(name), self=self, ep=profile))
1060+
10021061
self.profile_manager.profiles[name] = profile
10031062
profile.load_balancing_policy.populate(self, self.metadata.all_hosts())
10041063
# on_up after populate allows things like DCA LBP to choose default local dc

‎tests/unit/test_cluster.py‎

Lines changed: 103 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,9 @@
1616
except ImportError:
1717
import unittest # noqa
1818

19-
from mock import patch
19+
import logging
20+
21+
from mock import patch, Mock
2022

2123
from cassandra import ConsistencyLevel, DriverException, Timeout, Unavailable, RequestExecutionException, ReadTimeout, WriteTimeout, CoordinationFailure, ReadFailure, WriteFailure, FunctionFailure, AlreadyExists,\
2224
InvalidRequest, Unauthorized, AuthenticationFailed, OperationTimedOut, UnsupportedOperation, RequestValidationException, ConfigurationException
@@ -30,6 +32,8 @@
3032
from tests import connection_class
3133

3234

35+
log = logging.getLogger(__name__)
36+
3337
class ExceptionTypeTest(unittest.TestCase):
3438

3539
def test_exception_types(self):
@@ -364,28 +368,85 @@ def test_no_profiles_same_name(self):
364368
# cannot add a profile added dynamically
365369
self.assertRaises(ValueError, cluster.add_execution_profile, 'two', ExecutionProfile())
366370

367-
@mock_session_pools
368-
def test_warning_on_no_lbp_with_contact_points(self):
371+
def test_warning_on_no_lbp_with_contact_points_legacy_mode(self):
372+
"""
373+
Test that users are warned when they instantiate a Cluster object in
374+
legacy mode with contact points but no load-balancing policy.
375+
376+
@since 3.12.0
377+
@jira_ticket PYTHON-812
378+
@expected_result logs
379+
380+
@test_category configuration
381+
"""
382+
self._check_warning_on_no_lbp_with_contact_points(
383+
cluster_kwargs={'contact_points': ['127.0.0.1']}
384+
)
385+
386+
def test_warning_on_no_lbp_with_contact_points_profile_mode(self):
369387
"""
370-
Test that users are warned when they instantiate a Cluster object with
371-
contact points but no load-balancing policy.
388+
Test that users are warned when they instantiate a Cluster object in
389+
execution profile mode with contact points but no load-balancing
390+
policy.
372391
373392
@since 3.12.0
374393
@jira_ticket PYTHON-812
375394
@expected_result logs
376395
377396
@test_category configuration
378397
"""
398+
self._check_warning_on_no_lbp_with_contact_points(cluster_kwargs={
399+
'contact_points': ['127.0.0.1'],
400+
'execution_profiles': {EXEC_PROFILE_DEFAULT: ExecutionProfile()}
401+
})
402+
403+
@mock_session_pools
404+
def _check_warning_on_no_lbp_with_contact_points(self, cluster_kwargs):
379405
with patch('cassandra.cluster.log') as patched_logger:
380-
Cluster(contact_points=['127.0.0.1'])
406+
Cluster(**cluster_kwargs)
381407
patched_logger.warn.assert_called_once()
382408
warning_message = patched_logger.warn.call_args[0][0]
383-
self.assertIn('no load_balancing_policy', warning_message)
409+
self.assertIn('please specify a load-balancing policy', warning_message)
384410
self.assertIn("contact_points = ['127.0.0.1']", warning_message)
385-
self.assertIn('lbp = None', warning_message)
411+
412+
def test_no_warning_on_contact_points_with_lbp_legacy_mode(self):
413+
"""
414+
Test that users aren't warned when they instantiate a Cluster object
415+
with contact points and a load-balancing policy in legacy mode.
416+
417+
@since 3.12.0
418+
@jira_ticket PYTHON-812
419+
@expected_result no logs
420+
421+
@test_category configuration
422+
"""
423+
self._check_no_warning_on_contact_points_with_lbp({
424+
'contact_points': ['127.0.0.1'],
425+
'load_balancing_policy': object()
426+
})
427+
428+
def test_no_warning_on_contact_points_with_lbp_profiles_mode(self):
429+
"""
430+
Test that users aren't warned when they instantiate a Cluster object
431+
with contact points and a load-balancing policy in execution profile
432+
mode.
433+
434+
@since 3.12.0
435+
@jira_ticket PYTHON-812
436+
@expected_result no logs
437+
438+
@test_category configuration
439+
"""
440+
ep_with_lbp = ExecutionProfile(load_balancing_policy=object())
441+
self._check_no_warning_on_contact_points_with_lbp(cluster_kwargs={
442+
'contact_points': ['127.0.0.1'],
443+
'execution_profiles': {
444+
EXEC_PROFILE_DEFAULT: ep_with_lbp
445+
}
446+
})
386447

387448
@mock_session_pools
388-
def test_no_warning_on_contact_points_with_lbp(self):
449+
def _check_no_warning_on_contact_points_with_lbp(self, cluster_kwargs):
389450
"""
390451
Test that users aren't warned when they instantiate a Cluster object
391452
with contact points and a load-balancing policy.
@@ -397,5 +458,37 @@ def test_no_warning_on_contact_points_with_lbp(self):
397458
@test_category configuration
398459
"""
399460
with patch('cassandra.cluster.log') as patched_logger:
400-
Cluster(contact_points=['127.0.0.1'], load_balancing_policy=object())
461+
Cluster(**cluster_kwargs)
462+
patched_logger.warn.assert_not_called()
463+
464+
@mock_session_pools
465+
def test_warning_adding_no_lbp_ep_to_cluster_with_contact_points(self):
466+
ep_with_lbp = ExecutionProfile(load_balancing_policy=object())
467+
cluster = Cluster(
468+
contact_points=['127.0.0.1'],
469+
execution_profiles={EXEC_PROFILE_DEFAULT: ep_with_lbp})
470+
with patch('cassandra.cluster.log') as patched_logger:
471+
cluster.add_execution_profile(
472+
name='no_lbp',
473+
profile=ExecutionProfile()
474+
)
475+
476+
patched_logger.warn.assert_called_once()
477+
warning_message = patched_logger.warn.call_args[0][0]
478+
self.assertIn('no_lbp', warning_message)
479+
self.assertIn('trying to add', warning_message)
480+
self.assertIn('please specify a load-balancing policy', warning_message)
481+
482+
@mock_session_pools
483+
def test_no_warning_adding_lbp_ep_to_cluster_with_contact_points(self):
484+
ep_with_lbp = ExecutionProfile(load_balancing_policy=object())
485+
cluster = Cluster(
486+
contact_points=['127.0.0.1'],
487+
execution_profiles={EXEC_PROFILE_DEFAULT: ep_with_lbp})
488+
with patch('cassandra.cluster.log') as patched_logger:
489+
cluster.add_execution_profile(
490+
name='with_lbp',
491+
profile=ExecutionProfile(load_balancing_policy=Mock(name='lbp'))
492+
)
493+
401494
patched_logger.warn.assert_not_called()

0 commit comments

Comments
 (0)