Alter topic feature #448

vgvoleg · 2024-06-28T11:59:20Z

Pull request type

Please check the type of change your PR introduces:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Topic client has a new method alter_topic to update existing topics.

Other information

rekby · 2024-07-01T08:12:11Z

tests/topics/test_control_plane.py

@@ -39,6 +39,14 @@ async def test_describe_topic(self, driver, topic_path: str, topic_consumer):

        assert has_consumer

+    async def test_alter_topic(self, driver, topic_path):


The function needs a positive test too

rekby · 2024-07-01T08:17:45Z

ydb/_grpc/grpcwrapper/ydb_topic_public_types.py

+    set_read_from: Optional[datetime.datetime] = None
+    "All messages with smaller server written_at timestamp will be skipped."
+
+    set_supported_codecs: List[PublicCodec] = field(default_factory=lambda: list())


How will you distinct empty list and empty value (doesn't need to change)?

default for set_supported_codecs must be None

rekby · 2024-07-01T08:18:19Z

ydb/_grpc/grpcwrapper/ydb_topic_public_types.py

+    supported_codecs on topic must be contained inside this list.
+    """
+
+    alter_attributes: Dict[str, str] = field(default_factory=lambda: dict())


same question

rekby · 2024-07-01T08:20:43Z

ydb/_grpc/grpcwrapper/ydb_topic_public_types.py

+    set_partition_count_limit: Optional[int]
+    add_consumers: Optional[List[Union["PublicConsumer", str]]]
+    alter_consumers: Optional[List[Union["PublicAlterConsumer", str]]]
+    drop_consumers: Optional[List[str]]  # TODO: clarify


github-actions · 2024-07-02T10:57:42Z

🌋 Here are results of SLO test for python-sync:

Grafana Dashboard

rekby · 2024-07-02T14:55:57Z

ydb/_grpc/grpcwrapper/ydb_topic.py

+    set_metering_mode: "MeteringMode"
+
+    def to_proto(self) -> ydb_topic_pb2.AlterTopicRequest:
+        supported_codecs = self.set_supported_codecs.to_proto() if self.set_supported_codecs.codecs else None


Need distinct between empty list and none - for select between set empty codecs list and skip set the option

rekby · 2024-07-04T08:24:25Z

ydb/_grpc/grpcwrapper/ydb_topic.py

+    alter_attributes: Optional[Dict[str, str]]
+
+    def to_proto(self) -> ydb_topic_pb2.AlterConsumer:
+        supported_codecs = self.set_supported_codecs.to_proto() if self.set_supported_codecs else None


Explicit check for none

rekby · 2024-07-04T08:29:08Z

ydb/_grpc/grpcwrapper/ydb_topic.py

+    set_partition_count_limit: Optional[int]
+
+    @staticmethod
+    def from_proto(msg: ydb_topic_pb2.AlterPartitioningSettings) -> "AlterPartitioningSettings":


remove from_proto

rekby · 2024-07-04T08:29:31Z

ydb/_grpc/grpcwrapper/ydb_topic.py

+    path: str
+    add_consumers: List["Consumer"]
+    alter_partitioning_settings: AlterPartitioningSettings
+    set_retention_period: datetime.timedelta


rekby · 2024-07-04T14:20:25Z

ydb/_grpc/grpcwrapper/ydb_topic.py

+    set_metering_mode: Optional["MeteringMode"]
+
+    def to_proto(self) -> ydb_topic_pb2.AlterTopicRequest:
+        supported_codecs = self.set_supported_codecs.to_proto() if self.set_supported_codecs else None


explicit check if None

rekby · 2024-07-04T14:32:28Z

ydb/_grpc/grpcwrapper/ydb_topic.py

+
+        supported_codecs = None
+        if req.set_supported_codecs is not None:
+            supported_codecs = SupportedCodecs(req.set_supported_codecs)


Need appect all types from req.set_supported_codecs and drop deny from field order.

rekby · 2024-07-04T14:39:14Z

ydb/_grpc/grpcwrapper/ydb_topic_public_types.py

+    set_read_from: Optional[datetime.datetime] = None
+    "All messages with smaller server written_at timestamp will be skipped."
+
+    set_supported_codecs: List[PublicCodec] = field(default_factory=lambda: list())


default for set_supported_codecs must be None

rekby · 2024-07-04T14:39:36Z

ydb/_grpc/grpcwrapper/ydb_topic_public_types.py

+@dataclass
+class PublicAlterConsumer:
+    name: str
+    set_important: bool = False


rekby · 2024-07-05T14:35:12Z

ydb/_grpc/grpcwrapper/ydb_topic.py

@@ -69,6 +69,13 @@ def from_proto(msg: Optional[ydb_topic_pb2.SupportedCodecs]) -> "SupportedCodecs
    def to_public(self) -> List[ydb_topic_public_types.PublicCodec]:
        return list(map(Codec.to_public, self.codecs))

+    @staticmethod
+    def from_public(codecs: Optional[List[ydb_topic_public_types.PublicCodec]]) -> "SupportedCodecs":


Optional for result

Supported codec require Codec instead of PublicCodec

from_public need to accept list of PublicCodec or int

vgvoleg force-pushed the impl/alter_topic branch 2 times, most recently from d454b80 to 0164d38 Compare June 29, 2024 13:27

Alter topic feature

a4588fb

vgvoleg force-pushed the impl/alter_topic branch 4 times, most recently from ab66f36 to dab805a Compare July 1, 2024 08:11

rekby reviewed Jul 1, 2024

View reviewed changes

vgvoleg self-assigned this Jul 1, 2024

vgvoleg linked an issue Jul 1, 2024 that may be closed by this pull request

Add alter topic method #265

Closed

vgvoleg force-pushed the impl/alter_topic branch 2 times, most recently from f9280e2 to b246065 Compare July 2, 2024 10:47

vgvoleg force-pushed the impl/alter_topic branch 3 times, most recently from e96a901 to 7f63d1e Compare July 2, 2024 11:18

tests

badb27a

vgvoleg force-pushed the impl/alter_topic branch from 7f63d1e to badb27a Compare July 2, 2024 11:20

vgvoleg mentioned this pull request Jul 2, 2024

WIP: Alter topic docs for python sdk ydb-platform/ydb#6180

Merged

vgvoleg force-pushed the impl/alter_topic branch 4 times, most recently from cfb8a99 to c28216f Compare July 4, 2024 16:06

rekby reviewed Jul 5, 2024

View reviewed changes

vgvoleg force-pushed the impl/alter_topic branch 2 times, most recently from 74b7c06 to 72b0a6f Compare July 8, 2024 10:59

fix review comments

e8e1272

vgvoleg force-pushed the impl/alter_topic branch from 72b0a6f to e8e1272 Compare July 8, 2024 11:13

rekby approved these changes Jul 8, 2024

View reviewed changes

vgvoleg merged commit 3eeef84 into main Jul 8, 2024
11 checks passed

vgvoleg deleted the impl/alter_topic branch July 8, 2024 15:22

		@@ -39,6 +39,14 @@ async def test_describe_topic(self, driver, topic_path: str, topic_consumer):

		assert has_consumer

		async def test_alter_topic(self, driver, topic_path):

Alter topic feature #448

Alter topic feature #448

Uh oh!

Conversation

vgvoleg commented Jun 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull request type

What is the current behavior?

What is the new behavior?

Other information

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jul 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

vgvoleg commented Jun 28, 2024 •

edited

Loading

github-actions bot commented Jul 2, 2024 •

edited

Loading