fix review comments

Author: vgvoleg

ydb/_grpc/grpcwrapper/ydb_topic.py

@@ -49,7 +49,7 @@ def to_public(self) -> ydb_topic_public_types.PublicCodec:
 
 
 @dataclass
-class SupportedCodecs(IToProto, IFromProto, IToPublic):
+class SupportedCodecs(IToProto, IFromProto, IToPublic, IFromPublic):
     codecs: List[Codec]
 
     def to_proto(self) -> ydb_topic_pb2.SupportedCodecs:
@@ -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":
+        if codecs is None:
+            return None
+
+        return SupportedCodecs(codecs=codecs)
+
 
 @dataclass(order=True)
 class OffsetsRange(IFromProto, IToProto):
@@ -886,17 +893,21 @@ def from_proto(
 @dataclass
 class AlterConsumer(IToProto, IFromPublic):
     name: str
-    set_important: bool
-    set_read_from: datetime.datetime
-    set_supported_codecs: SupportedCodecs
-    alter_attributes: Dict[str, str]
+    set_important: Optional[bool]
+    set_read_from: Optional[datetime.datetime]
+    set_supported_codecs: Optional[SupportedCodecs]
+    alter_attributes: Optional[Dict[str, str]]
 
     def to_proto(self) -> ydb_topic_pb2.AlterConsumer:
+        supported_codecs = None
+        if self.set_supported_codecs is not None:
+            supported_codecs = self.set_supported_codecs.to_proto()
+
         return ydb_topic_pb2.AlterConsumer(
             name=self.name,
             set_important=self.set_important,
             set_read_from=proto_timestamp_from_datetime(self.set_read_from),
-            set_supported_codecs=self.set_supported_codecs.to_proto(),
+            set_supported_codecs=supported_codecs,
             alter_attributes=self.alter_attributes,
         )
 
@@ -905,13 +916,11 @@ def from_public(alter_consumer: ydb_topic_public_types.PublicAlterConsumer) -> A
         if not alter_consumer:
             return None
 
-        supported_codecs = alter_consumer.set_supported_codecs if alter_consumer.set_supported_codecs else []
-
         return AlterConsumer(
             name=alter_consumer.name,
             set_important=alter_consumer.set_important,
             set_read_from=alter_consumer.set_read_from,
-            set_supported_codecs=SupportedCodecs(codecs=supported_codecs),
+            set_supported_codecs=SupportedCodecs.from_public(alter_consumer.set_supported_codecs),
             alter_attributes=alter_consumer.alter_attributes,
         )
 
@@ -936,16 +945,9 @@ def to_proto(self) -> ydb_topic_pb2.PartitioningSettings:
 
 
 @dataclass
-class AlterPartitioningSettings(IToProto, IFromProto):
-    set_min_active_partitions: int
-    set_partition_count_limit: int
-
-    @staticmethod
-    def from_proto(msg: ydb_topic_pb2.AlterPartitioningSettings) -> "AlterPartitioningSettings":
-        return AlterPartitioningSettings(
-            set_min_active_partitions=msg.set_min_active_partitions,
-            set_partition_count_limit=msg.set_partition_count_limit,
-        )
+class AlterPartitioningSettings(IToProto):
+    set_min_active_partitions: Optional[int]
+    set_partition_count_limit: Optional[int]
 
     def to_proto(self) -> ydb_topic_pb2.AlterPartitioningSettings:
         return ydb_topic_pb2.AlterPartitioningSettings(
@@ -1050,20 +1052,22 @@ class CreateTopicResult:
 @dataclass
 class AlterTopicRequest(IToProto, IFromPublic):
     path: str
-    add_consumers: List["Consumer"]
-    alter_partitioning_settings: AlterPartitioningSettings
-    set_retention_period: datetime.timedelta
-    set_retention_storage_mb: int
-    set_supported_codecs: SupportedCodecs
-    set_partition_write_burst_bytes: typing.Optional[int]
-    set_partition_write_speed_bytes_per_second: typing.Optional[int]
-    alter_attributes: Dict[str, str]
-    alter_consumers: List[AlterConsumer]
-    drop_consumers: List[str]
-    set_metering_mode: "MeteringMode"
+    add_consumers: Optional[List["Consumer"]]
+    alter_partitioning_settings: Optional[AlterPartitioningSettings]
+    set_retention_period: Optional[datetime.timedelta]
+    set_retention_storage_mb: Optional[int]
+    set_supported_codecs: Optional[SupportedCodecs]
+    set_partition_write_burst_bytes: Optional[int]
+    set_partition_write_speed_bytes_per_second: Optional[int]
+    alter_attributes: Optional[Dict[str, str]]
+    alter_consumers: Optional[List[AlterConsumer]]
+    drop_consumers: Optional[List[str]]
+    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.codecs else None
+        supported_codecs = None
+        if self.set_supported_codecs is not None:
+            supported_codecs = self.set_supported_codecs.to_proto()
 
         return ydb_topic_pb2.AlterTopicRequest(
             path=self.path,
@@ -1098,8 +1102,6 @@ def from_public(req: ydb_topic_public_types.AlterTopicRequestParams) -> AlterTop
 
         drop_consumers = req.drop_consumers if req.drop_consumers else []
 
-        supported_codecs = req.set_supported_codecs if req.set_supported_codecs else []
-
         return AlterTopicRequest(
             path=req.path,
             alter_partitioning_settings=AlterPartitioningSettings(
@@ -1109,9 +1111,7 @@ def from_public(req: ydb_topic_public_types.AlterTopicRequestParams) -> AlterTop
             add_consumers=add_consumers,
             set_retention_period=req.set_retention_period,
             set_retention_storage_mb=req.set_retention_storage_mb,
-            set_supported_codecs=SupportedCodecs(
-                codecs=supported_codecs,
-            ),
+            set_supported_codecs=SupportedCodecs.from_public(req.set_supported_codecs),
             set_partition_write_burst_bytes=req.set_partition_write_burst_bytes,
             set_partition_write_speed_bytes_per_second=req.set_partition_write_speed_bytes_per_second,
             alter_attributes=req.alter_attributes,
@@ -1121,11 +1121,6 @@ def from_public(req: ydb_topic_public_types.AlterTopicRequestParams) -> AlterTop
         )
 
 
-@dataclass
-class AlterTopicResult:
-    pass
-
-
 @dataclass
 class DescribeTopicRequest:
     path: str

ydb/_grpc/grpcwrapper/ydb_topic_public_types.py

@@ -93,7 +93,7 @@ class PublicConsumer:
 @dataclass
 class PublicAlterConsumer:
     name: str
-    set_important: bool = False
+    set_important: Optional[bool] = None
     """
     Consumer may be marked as 'important'. It means messages for this consumer will never expire due to retention.
     User should take care that such consumer never stalls, to prevent running out of disk space.
@@ -102,13 +102,13 @@ class PublicAlterConsumer:
     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())
+    set_supported_codecs: Optional[List[PublicCodec]] = None
     """
     List of supported codecs by this consumer.
     supported_codecs on topic must be contained inside this list.
     """
 
-    alter_attributes: Dict[str, str] = field(default_factory=lambda: dict())
+    alter_attributes: Optional[Dict[str, str]] = None
     "Attributes of consumer"
 
 

ydb/_grpc/grpcwrapper/ydb_topic_test.py

@@ -60,27 +60,32 @@ def test_alter_topic_request_from_public_to_proto():
 
     msg_dict = MessageToDict(request_proto, preserving_proto_field_name=True)
 
-    assert msg_dict["path"] == params["path"]
-    assert len(msg_dict["add_consumers"]) == len(params["add_consumers"])
-    assert len(msg_dict["alter_consumers"]) == len(params["alter_consumers"])
-    assert len(msg_dict["drop_consumers"]) == len(params["drop_consumers"])
-    assert msg_dict["alter_attributes"] == params["alter_attributes"]
-
-    assert (
-        int(msg_dict["alter_partitioning_settings"]["set_min_active_partitions"]) == params["set_min_active_partitions"]
-    )
-    assert (
-        int(msg_dict["alter_partitioning_settings"]["set_partition_count_limit"]) == params["set_partition_count_limit"]
-    )
-
-    assert int(msg_dict["set_partition_write_burst_bytes"]) == params["set_partition_write_burst_bytes"]
-    assert (
-        int(msg_dict["set_partition_write_speed_bytes_per_second"])
-        == params["set_partition_write_speed_bytes_per_second"]
-    )
-    assert msg_dict["set_retention_period"] == str(int(params["set_retention_period"].total_seconds())) + "s"
-    assert int(msg_dict["set_retention_storage_mb"]) == params["set_retention_storage_mb"]
-
-    assert msg_dict["set_metering_mode"] == "METERING_MODE_RESERVED_CAPACITY"
+    expected_dict = {
+        "path": "topic_name",
+        "alter_partitioning_settings": {
+            "set_min_active_partitions": "2",
+            "set_partition_count_limit": "4"
+        },
+        "set_retention_period": "2419200s",
+        "set_retention_storage_mb": "4",
+        "set_supported_codecs": {
+            "codecs": [1, 2]
+        },
+        "set_partition_write_speed_bytes_per_second": "15",
+        "set_partition_write_burst_bytes": "8",
+        "alter_attributes": {
+            "key": "value"
+        },
+        "add_consumers": [
+            {"name": "new_consumer_1", "supported_codecs": {}},
+            {"name": "new_consumer_2", "supported_codecs": {}}
+        ],
+        "drop_consumers": ["redundant_consumer"],
+        "alter_consumers": [
+            {"name": "old_consumer_1"},
+            {"name": "old_consumer_2"}
+        ],
+        "set_metering_mode": "METERING_MODE_RESERVED_CAPACITY"
+    }
 
-    assert msg_dict["set_supported_codecs"]["codecs"] == params["set_supported_codecs"]
+    assert msg_dict == expected_dict