From 62ad5fd6e6b15ab24c681b665cfd49954c9a87d9 Mon Sep 17 00:00:00 2001 From: Emanuele Sabellico Date: Tue, 7 Jun 2022 16:28:54 +0200 Subject: [PATCH 1/5] documentation fixes --- docs/index.rst | 12 ++++++------ src/confluent_kafka/admin/__init__.py | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/docs/index.rst b/docs/index.rst index a1a7a9043..30f09d988 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -136,9 +136,9 @@ ResourceType .. _pythonclient_resource_pattern_type: -************** +******************* ResourcePatternType -************** +******************* .. autoclass:: confluent_kafka.admin.ResourcePatternType :members: @@ -154,9 +154,9 @@ AclOperation .. _pythonclient_acl_permission_type: -************** +***************** AclPermissionType -************** +***************** .. autoclass:: confluent_kafka.admin.AclPermissionType :members: @@ -172,9 +172,9 @@ AclBinding .. _pythonclient_acl_binding_filter: -************** +**************** AclBindingFilter -************** +**************** .. autoclass:: confluent_kafka.admin.AclBindingFilter :members: diff --git a/src/confluent_kafka/admin/__init__.py b/src/confluent_kafka/admin/__init__.py index fa2b2c30e..09775ebbb 100644 --- a/src/confluent_kafka/admin/__init__.py +++ b/src/confluent_kafka/admin/__init__.py @@ -395,7 +395,7 @@ def describe_acls(self, acl_binding_filter, **kwargs): :param AclBindingFilter acl_binding_filter: a filter with attributes that must match. String attributes match exact values or any string if set to None. - Enums attributes match exact values or any value if ending with `_ANY`. + Enums attributes match exact values or any value if equal to `ANY`. If :class:`ResourcePatternType` is set to :attr:`ResourcePatternType.MATCH` returns all the ACL bindings with :attr:`ResourcePatternType.LITERAL`, :attr:`ResourcePatternType.WILDCARD` or :attr:`ResourcePatternType.PREFIXED` pattern @@ -426,7 +426,7 @@ def delete_acls(self, acl_binding_filters, **kwargs): :param list(AclBindingFilter) acl_binding_filters: a list of ACL binding filters to match ACLs to delete. String attributes match exact values or any string if set to None. - Enums attributes match exact values or any value if ending with `_ANY`. + Enums attributes match exact values or any value if equal to `ANY`. If :class:`ResourcePatternType` is set to :attr:`ResourcePatternType.MATCH` deletes all the ACL bindings with :attr:`ResourcePatternType.LITERAL`, :attr:`ResourcePatternType.WILDCARD` or :attr:`ResourcePatternType.PREFIXED` From 4d25760bfe407dd176811b9c0049d683e7e5f9e7 Mon Sep 17 00:00:00 2001 From: Emanuele Sabellico Date: Tue, 14 Jun 2022 11:37:40 +0200 Subject: [PATCH 2/5] fix: raised ValueError if equal ACL bindings or ACL binding filters are passed. --- examples/adminapi.py | 12 ++++++++++-- src/confluent_kafka/admin/__init__.py | 12 ++++++++++-- tests/test_Admin.py | 6 ++++++ 3 files changed, 26 insertions(+), 4 deletions(-) diff --git a/examples/adminapi.py b/examples/adminapi.py index 657d41c51..2bc0577be 100755 --- a/examples/adminapi.py +++ b/examples/adminapi.py @@ -152,7 +152,11 @@ def example_create_acls(a, args): ) ] - fs = a.create_acls(acl_bindings, request_timeout=10) + try: + fs = a.create_acls(acl_bindings, request_timeout=10) + except ValueError as e: + print(f"Invalid input: {e}") + return # Wait for operation to finish. for res, f in fs.items(): @@ -237,7 +241,11 @@ def example_delete_acls(a, args): ) ] - fs = a.delete_acls(acl_binding_filters, request_timeout=10) + try: + fs = a.delete_acls(acl_binding_filters, request_timeout=10) + except ValueError as e: + print(f"Invalid input: {e}") + return # Wait for operation to finish. for res, f in fs.items(): diff --git a/src/confluent_kafka/admin/__init__.py b/src/confluent_kafka/admin/__init__.py index 09775ebbb..93c7512bc 100644 --- a/src/confluent_kafka/admin/__init__.py +++ b/src/confluent_kafka/admin/__init__.py @@ -181,6 +181,10 @@ def _make_futures(futmap_keys, class_check, make_result_fn): return f, futmap + @staticmethod + def _has_duplicated(items): + return len(set(items)) != len(items) + def create_topics(self, new_topics, **kwargs): """ Create one or more new topics. @@ -365,7 +369,7 @@ def create_acls(self, acls, **kwargs): """ Create one or more ACL bindings. - :param list(AclBinding) acls: A list of ACL binding specifications (:class:`.AclBinding`) + :param list(AclBinding) acls: A list of unique ACL binding specifications (:class:`.AclBinding`) to create. :param float request_timeout: The overall request timeout in seconds, including broker lookup, request transmission, operation time @@ -380,6 +384,8 @@ def create_acls(self, acls, **kwargs): :raises TypeException: Invalid input. :raises ValueException: Invalid input. """ + if AdminClient._has_duplicated(acls): + raise ValueError("pass only different ACL bindings") f, futmap = AdminClient._make_futures(acls, AclBinding, AdminClient._make_acls_result) @@ -423,7 +429,7 @@ def delete_acls(self, acl_binding_filters, **kwargs): """ Delete ACL bindings matching one or more ACL binding filters. - :param list(AclBindingFilter) acl_binding_filters: a list of ACL binding filters + :param list(AclBindingFilter) acl_binding_filters: a list of unique ACL binding filters to match ACLs to delete. String attributes match exact values or any string if set to None. Enums attributes match exact values or any value if equal to `ANY`. @@ -444,6 +450,8 @@ def delete_acls(self, acl_binding_filters, **kwargs): :raises TypeException: Invalid input. :raises ValueException: Invalid input. """ + if AdminClient._has_duplicated(acl_binding_filters): + raise ValueError("pass only different ACL binding filters") f, futmap = AdminClient._make_futures(acl_binding_filters, AclBindingFilter, AdminClient._make_acls_result) diff --git a/tests/test_Admin.py b/tests/test_Admin.py index 8e44900a5..f50685bd9 100644 --- a/tests/test_Admin.py +++ b/tests/test_Admin.py @@ -347,6 +347,9 @@ def test_create_acls_api(): with pytest.raises(ValueError): a.create_acls([None, acl_binding1]) + with pytest.raises(ValueError): + a.create_acls([acl_binding1, acl_binding1]) + fs = a.create_acls([acl_binding1, acl_binding2]) with pytest.raises(KafkaException): for f in fs.values(): @@ -391,6 +394,9 @@ def test_delete_acls_api(): with pytest.raises(ValueError): a.delete_acls([None, acl_binding_filter1]) + with pytest.raises(ValueError): + a.delete_acls([acl_binding_filter1, acl_binding_filter1]) + fs = a.delete_acls([acl_binding_filter1, acl_binding_filter2]) with pytest.raises(KafkaException): for f in concurrent.futures.as_completed(iter(fs.values())): From 076e282bfd05c1284021a90e9f9c81951ce1883a Mon Sep 17 00:00:00 2001 From: Emanuele Sabellico Date: Tue, 14 Jun 2022 20:19:33 +0200 Subject: [PATCH 3/5] definition of variable in the outer scope --- examples/adminapi.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/examples/adminapi.py b/examples/adminapi.py index 2bc0577be..e08a981de 100755 --- a/examples/adminapi.py +++ b/examples/adminapi.py @@ -152,12 +152,17 @@ def example_create_acls(a, args): ) ] + fs = None try: fs = a.create_acls(acl_bindings, request_timeout=10) except ValueError as e: print(f"Invalid input: {e}") return + if fs is None: + print("Unknown error: fs is None") + return + # Wait for operation to finish. for res, f in fs.items(): try: @@ -241,11 +246,16 @@ def example_delete_acls(a, args): ) ] + fs = None try: fs = a.delete_acls(acl_binding_filters, request_timeout=10) except ValueError as e: print(f"Invalid input: {e}") return + + if fs is None: + print("Unknown error: fs is None") + return # Wait for operation to finish. for res, f in fs.items(): From 0f702ce74ca5d53bfda557773f23fe35a3a6d7a4 Mon Sep 17 00:00:00 2001 From: Emanuele Sabellico Date: Tue, 14 Jun 2022 21:02:40 +0200 Subject: [PATCH 4/5] removed None check --- examples/adminapi.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/examples/adminapi.py b/examples/adminapi.py index e08a981de..d7f2c643f 100755 --- a/examples/adminapi.py +++ b/examples/adminapi.py @@ -159,10 +159,6 @@ def example_create_acls(a, args): print(f"Invalid input: {e}") return - if fs is None: - print("Unknown error: fs is None") - return - # Wait for operation to finish. for res, f in fs.items(): try: @@ -252,10 +248,6 @@ def example_delete_acls(a, args): except ValueError as e: print(f"Invalid input: {e}") return - - if fs is None: - print("Unknown error: fs is None") - return # Wait for operation to finish. for res, f in fs.items(): From 5d3508910eb383458796a06862ba4dc798908ec3 Mon Sep 17 00:00:00 2001 From: Emanuele Sabellico Date: Wed, 15 Jun 2022 11:52:30 +0200 Subject: [PATCH 5/5] grammar and messages --- examples/adminapi.py | 6 ++---- src/confluent_kafka/admin/__init__.py | 10 +++++----- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/examples/adminapi.py b/examples/adminapi.py index d7f2c643f..74012e8f3 100755 --- a/examples/adminapi.py +++ b/examples/adminapi.py @@ -152,11 +152,10 @@ def example_create_acls(a, args): ) ] - fs = None try: fs = a.create_acls(acl_bindings, request_timeout=10) except ValueError as e: - print(f"Invalid input: {e}") + print(f"create_acls() failed: {e}") return # Wait for operation to finish. @@ -242,11 +241,10 @@ def example_delete_acls(a, args): ) ] - fs = None try: fs = a.delete_acls(acl_binding_filters, request_timeout=10) except ValueError as e: - print(f"Invalid input: {e}") + print(f"delete_acls() failed: {e}") return # Wait for operation to finish. diff --git a/src/confluent_kafka/admin/__init__.py b/src/confluent_kafka/admin/__init__.py index 93c7512bc..965f3e268 100644 --- a/src/confluent_kafka/admin/__init__.py +++ b/src/confluent_kafka/admin/__init__.py @@ -182,7 +182,7 @@ def _make_futures(futmap_keys, class_check, make_result_fn): return f, futmap @staticmethod - def _has_duplicated(items): + def _has_duplicates(items): return len(set(items)) != len(items) def create_topics(self, new_topics, **kwargs): @@ -384,8 +384,8 @@ def create_acls(self, acls, **kwargs): :raises TypeException: Invalid input. :raises ValueException: Invalid input. """ - if AdminClient._has_duplicated(acls): - raise ValueError("pass only different ACL bindings") + if AdminClient._has_duplicates(acls): + raise ValueError("duplicate ACL bindings not allowed") f, futmap = AdminClient._make_futures(acls, AclBinding, AdminClient._make_acls_result) @@ -450,8 +450,8 @@ def delete_acls(self, acl_binding_filters, **kwargs): :raises TypeException: Invalid input. :raises ValueException: Invalid input. """ - if AdminClient._has_duplicated(acl_binding_filters): - raise ValueError("pass only different ACL binding filters") + if AdminClient._has_duplicates(acl_binding_filters): + raise ValueError("duplicate ACL binding filters not allowed") f, futmap = AdminClient._make_futures(acl_binding_filters, AclBindingFilter, AdminClient._make_acls_result)