slbotbm commented on code in PR #4017:
URL: https://github.com/apache/iggy/pull/4017#discussion_r3940318938
##########
foreign/python/apache_iggy.pyi:
##########
@@ -1194,6 +1194,56 @@ class IggyClient:
Raises:
RuntimeError: If an identifier is invalid or the request fails.
"""
+ def create_partitions(
+ self,
+ stream_id: builtins.str | builtins.int,
+ topic_id: builtins.str | builtins.int,
+ partitions_count: builtins.int,
+ ) -> collections.abc.Awaitable[None]:
+ r"""
+ Create partitions for a topic. New partitions use consecutive,
zero-based IDs
+ after the current maximum; IDs removed by deletion can be reused.
Existing
+ consumer groups leave them unassigned until their next rebalance.
+
+ Args:
+ stream_id: Stream identifier as `str | int`.
+ topic_id: Topic identifier as `str | int`.
+ partitions_count: Number of partitions to create as `int`; must be
+ 1..=1000.
Review Comment:
same for this
##########
foreign/python/src/client.rs:
##########
@@ -57,6 +57,10 @@ pub struct IggyClient {
inner: Arc<RustIggyClient>,
}
+fn to_runtime_error(error: impl ToString) -> PyErr {
+ PyErr::new::<pyo3::exceptions::PyRuntimeError, _>(error.to_string())
Review Comment:
No need for a new function. Inline this at call sites.
##########
foreign/python/apache_iggy.pyi:
##########
Review Comment:
In the docs, also mention if there are any permission-related restrictions
for these two functions.
##########
foreign/python/apache_iggy.pyi:
##########
@@ -1194,6 +1194,56 @@ class IggyClient:
Raises:
RuntimeError: If an identifier is invalid or the request fails.
"""
+ def create_partitions(
+ self,
+ stream_id: builtins.str | builtins.int,
+ topic_id: builtins.str | builtins.int,
+ partitions_count: builtins.int,
+ ) -> collections.abc.Awaitable[None]:
+ r"""
+ Create partitions for a topic. New partitions use consecutive,
zero-based IDs
+ after the current maximum; IDs removed by deletion can be reused.
Existing
+ consumer groups leave them unassigned until their next rebalance.
+
+ Args:
+ stream_id: Stream identifier as `str | int`.
+ topic_id: Topic identifier as `str | int`.
+ partitions_count: Number of partitions to create as `int`; must be
+ 1..=1000.
+
+ Returns:
+ An awaitable that resolves to `None` when the partitions are
created.
+
+ Raises:
+ ValueError: If an identifier is invalid.
+ OverflowError: If `partitions_count` is outside the unsigned
32-bit range.
+ RuntimeError: If the request fails.
+ """
+ def delete_partitions(
+ self,
+ stream_id: builtins.str | builtins.int,
+ topic_id: builtins.str | builtins.int,
+ partitions_count: builtins.int,
+ ) -> collections.abc.Awaitable[None]:
+ r"""
+ Delete the last partitions from a topic, including all messages stored
in them.
+ Consumer groups are rebalanced away from the removed partitions.
+
+ Args:
+ stream_id: Stream identifier as `str | int`.
+ topic_id: Topic identifier as `str | int`.
+ partitions_count: Number of partitions to delete as `int` from the
end of
+ the topic; must be 1..=1000 and no greater than its current
count.
Review Comment:
`1..=1000` is rust lingo, not suitable for python. Write it in a way python
programmers will understand.
##########
foreign/python/apache_iggy.pyi:
##########
@@ -1194,6 +1194,56 @@ class IggyClient:
Raises:
RuntimeError: If an identifier is invalid or the request fails.
"""
+ def create_partitions(
+ self,
+ stream_id: builtins.str | builtins.int,
+ topic_id: builtins.str | builtins.int,
+ partitions_count: builtins.int,
+ ) -> collections.abc.Awaitable[None]:
+ r"""
+ Create partitions for a topic. New partitions use consecutive,
zero-based IDs
+ after the current maximum; IDs removed by deletion can be reused.
Existing
+ consumer groups leave them unassigned until their next rebalance.
Review Comment:
> Existing consumer groups leave them unassigned until their next rebalance.
This documents wrong behaviour. A clearer description would be
> Creating partitions immediately rebalances existing consumer groups,
redistributing all topic partitions among their current members and advancing
the group generation.
##########
foreign/python/apache_iggy.pyi:
##########
Review Comment:
Also add Python integration tests for authorization. For both
create_partitions and delete_partitions, verify that a logged in user without
manage_topic permission receives Unauthorized and the partition count remains
unchanged. Also verify that a user granted topic-level manage_topic can perform
both operations. A topic-scoping check, proving permission for topic A does not
authorize topic B, would be valuable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
