You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iggy.apache.org by "mattp5657 (via GitHub)" <gi...@apache.org> on 2026/07/07 00:01:32 UTC
[PR] Feat/python sdk topic details fields [iggy]
mattp5657 opened a new pull request, #3623:
URL: https://github.com/apache/iggy/pull/3623
## Which issue does this PR address?
Closes #3577
## Rationale
Continuation of #3572. That PR exposed `Topic`/`TopicDetails` basics but left several fields returned by the server unavailable in Python, so callers couldn't inspect a topic's expiry, size, max size, or its partitions without another SDK.
## What changed?
`Topic` and `TopicDetails` previously exposed only `id`, `name`, `partitions_count`, `compression_algorithm`, and `replication_factor`, leaving `created_at`, `size`, `message_expiry`, `max_topic_size`, and per-partition data unavailable in Python.
Both classes now expose `created_at`, `size`, `message_expiry` (new `IggyExpiry` enum), and `max_topic_size` (new `MaxTopicSize` enum). `TopicDetails` also gains `partitions`, a list of new `Partition` objects. `Partition` (`core/common`) now derives `Clone` to support building that list.
## Local Execution
- Passed
- Pre-commit hooks ran
## AI Usage
<!--
If AI tools were used, please answer:
1. Which tools? (e.g., GitHub Copilot, Claude, ChatGPT)
2. Scope of usage? (e.g., autocomplete, generated functions, entire implementation)
3. How did you verify the generated code works correctly?
4. Can you explain every line of the code if asked?
If no AI tools were used, write "None" or delete this section.
-->
Claude was used to help generate and review this PR.
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(python): expose remaining Topic/TopicDetails fields and partitions [iggy]
Posted by "hubcio (via GitHub)" <gi...@apache.org>.
hubcio merged PR #3623:
URL: https://github.com/apache/iggy/pull/3623
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(python): expose remaining Topic/TopicDetails fields and partitions [iggy]
Posted by "mattp5657 (via GitHub)" <gi...@apache.org>.
mattp5657 commented on PR #3623:
URL: https://github.com/apache/iggy/pull/3623#issuecomment-5049431425
I think this test might be flaky. I'll create an issue to look into it.
`server::message_cleanup::message_cleanup::expiry_respects_consumer_offset_expects`
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(python): expose remaining Topic/TopicDetails fields and partitions [iggy]
Posted by "ryerraguntla (via GitHub)" <gi...@apache.org>.
ryerraguntla commented on code in PR #3623:
URL: https://github.com/apache/iggy/pull/3623#discussion_r3623598469
##########
foreign/python/src/topic.rs:
##########
@@ -15,9 +15,149 @@
// specific language governing permissions and limitations
// under the License.
-use iggy::prelude::{Topic as RustTopic, TopicDetails as RustTopicDetails};
+use std::time::Duration;
+
+use iggy::prelude::{
+ IggyByteSize, IggyExpiry as RustIggyExpiry, MaxTopicSize as RustMaxTopicSize,
+ Partition as RustPartition, Topic as RustTopic, TopicDetails as RustTopicDetails,
+};
+use pyo3::exceptions::PyValueError;
use pyo3::prelude::*;
-use pyo3_stub_gen::derive::{gen_stub_pyclass, gen_stub_pymethods};
+use pyo3::types::PyDelta;
+use pyo3_stub_gen::derive::{gen_stub_pyclass, gen_stub_pyclass_complex_enum, gen_stub_pymethods};
+
+use crate::consumer::py_delta_to_iggy_duration;
+
+/// The expiry of the messages in a topic.
+#[gen_stub_pyclass_complex_enum]
+#[pyclass]
+pub enum IggyExpiry {
+ /// Use the message expiry configured on the server for this topic,
+ /// rather than an explicit value set by the client.
+ ServerDefault(),
+ /// Expire messages this long after they are appended to the topic.
+ ///
+ /// `duration` must be greater than zero and less than the maximum
+ /// microsecond count a `u64` can hold (about 584,542 years): those two
+ /// values are reserved on the wire for `ServerDefault` and `NeverExpire`
+ /// respectively, so a `duration` at either boundary raises `ValueError`
+ /// when passed to `create_topic`/`update_topic`. A negative `timedelta`
+ /// also raises `ValueError`.
+ ExpireDuration { duration: Py<PyDelta> },
+ /// Retain messages indefinitely; they never expire.
+ NeverExpire(),
+}
+
+impl From<RustIggyExpiry> for IggyExpiry {
+ fn from(expiry: RustIggyExpiry) -> Self {
+ match expiry {
+ RustIggyExpiry::ServerDefault => IggyExpiry::ServerDefault(),
+ RustIggyExpiry::ExpireDuration(duration) => IggyExpiry::ExpireDuration {
+ duration: iggy_duration_to_py_delta(duration.get_duration()),
+ },
+ RustIggyExpiry::NeverExpire => IggyExpiry::NeverExpire(),
+ }
+ }
+}
+
+impl TryFrom<&IggyExpiry> for RustIggyExpiry {
+ type Error = PyErr;
+
+ fn try_from(expiry: &IggyExpiry) -> PyResult<Self> {
+ Ok(match expiry {
+ IggyExpiry::ServerDefault() => RustIggyExpiry::ServerDefault,
+ IggyExpiry::ExpireDuration { duration } => {
+ let iggy_duration = py_delta_to_iggy_duration(duration)?;
+ if iggy_duration.is_zero() {
+ return Err(PyValueError::new_err(
+ "duration must be greater than zero and less than the maximum \
+ representable microsecond count; those values are reserved for \
+ IggyExpiry.ServerDefault() and IggyExpiry.NeverExpire() respectively"
+ .to_string(),
+ ));
+ }
+ if iggy_duration.get_duration().as_micros() >= u64::MAX as u128 {
+ return Err(PyValueError::new_err(
+ "duration must be greater than zero and less than the maximum \
+ representable microsecond count; those values are reserved for \
+ IggyExpiry.ServerDefault() and IggyExpiry.NeverExpire() respectively"
+ .to_string(),
+ ));
+ }
+ RustIggyExpiry::ExpireDuration(iggy_duration)
+ }
+ IggyExpiry::NeverExpire() => RustIggyExpiry::NeverExpire,
+ })
+ }
+}
+
+fn iggy_duration_to_py_delta(duration: Duration) -> Py<PyDelta> {
Review Comment:
topic.rs:94-106 — iggy_duration_to_py_delta uses .expect on PyDelta::new; get_topic/message_expiry read path can abort on corrupt wire duration. **Fix**: map PyErr, raise ValueError.
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(python): expose remaining Topic/TopicDetails fields and partitions [iggy]
Posted by "mattp5657 (via GitHub)" <gi...@apache.org>.
mattp5657 closed pull request #3623: feat(python): expose remaining Topic/TopicDetails fields and partitions
URL: https://github.com/apache/iggy/pull/3623
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(python): expose remaining Topic/TopicDetails fields and partitions [iggy]
Posted by "mattp5657 (via GitHub)" <gi...@apache.org>.
mattp5657 commented on PR #3623:
URL: https://github.com/apache/iggy/pull/3623#issuecomment-5025519623
I have addressed all the comments and added the following to the PR:
`create_topic`'s `message_expiry` and `max_topic_size` parameters both
change type on this branch, and both changes are breaking relative to
the last released version (`0.8.0`):
| Parameter | `0.8.0` (released) | This branch |
|------------------|-----------------------------|-----------------------|
| `message_expiry` | `datetime.timedelta \| None` | `IggyExpiry \| None` |
| `max_topic_size` | `builtins.int \| None` | `MaxTopicSize \| None` |
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(python): expose remaining Topic/TopicDetails fields and partitions [iggy]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #3623:
URL: https://github.com/apache/iggy/pull/3623#issuecomment-4898766641
## [Codecov](https://app.codecov.io/gh/apache/iggy/pull/3623?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 72.40%. Comparing base ([`354be5c`](https://app.codecov.io/gh/apache/iggy/commit/354be5c928ecdf1096e92f6be33970b286105ac7?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`1c7445c`](https://app.codecov.io/gh/apache/iggy/commit/1c7445ce0220c3e18d1028772db132f45cfb25c5?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #3623 +/- ##
============================================
- Coverage 74.11% 72.40% -1.71%
Complexity 937 937
============================================
Files 1262 1199 -63
Lines 132757 128070 -4687
Branches 108647 108558 -89
============================================
- Hits 98396 92735 -5661
- Misses 31273 32334 +1061
+ Partials 3088 3001 -87
```
| [Components](https://app.codecov.io/gh/apache/iggy/pull/3623/components?src=pr&el=components&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [Rust Core](https://app.codecov.io/gh/apache/iggy/pull/3623/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `74.51% <ø> (ø)` | |
| [Java SDK](https://app.codecov.io/gh/apache/iggy/pull/3623/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `62.44% <ø> (ø)` | |
| [C# SDK](https://app.codecov.io/gh/apache/iggy/pull/3623/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `72.10% <ø> (ø)` | |
| [Python SDK](https://app.codecov.io/gh/apache/iggy/pull/3623/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `91.65% <ø> (ø)` | |
| [PHP SDK](https://app.codecov.io/gh/apache/iggy/pull/3623/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `84.29% <ø> (ø)` | |
| [Node SDK](https://app.codecov.io/gh/apache/iggy/pull/3623/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `87.27% <ø> (-4.08%)` | :arrow_down: |
| [Go SDK](https://app.codecov.io/gh/apache/iggy/pull/3623/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `16.50% <ø> (-25.78%)` | :arrow_down: |
| [Files with missing lines](https://app.codecov.io/gh/apache/iggy/pull/3623?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [foreign/python/src/lib.rs](https://app.codecov.io/gh/apache/iggy/pull/3623?src=pr&el=tree&filepath=foreign%2Fpython%2Fsrc%2Flib.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Zm9yZWlnbi9weXRob24vc3JjL2xpYi5ycw==) | `100.00% <ø> (ø)` | |
| [foreign/python/src/topic.rs](https://app.codecov.io/gh/apache/iggy/pull/3623?src=pr&el=tree&filepath=foreign%2Fpython%2Fsrc%2Ftopic.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Zm9yZWlnbi9weXRob24vc3JjL3RvcGljLnJz) | `95.00% <ø> (ø)` | |
... and [123 files with indirect coverage changes](https://app.codecov.io/gh/apache/iggy/pull/3623/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(python): expose remaining Topic/TopicDetails fields and partitions [iggy]
Posted by "mattp5657 (via GitHub)" <gi...@apache.org>.
mattp5657 commented on PR #3623:
URL: https://github.com/apache/iggy/pull/3623#issuecomment-5025520262
/ready
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(python): expose remaining Topic/TopicDetails fields and partitions [iggy]
Posted by "hubcio (via GitHub)" <gi...@apache.org>.
hubcio commented on code in PR #3623:
URL: https://github.com/apache/iggy/pull/3623#discussion_r3586105959
##########
foreign/python/src/consumer.rs:
##########
@@ -517,11 +519,19 @@ impl PyStubType for AutoCommitAfter {
}
}
-pub fn py_delta_to_iggy_duration(delta1: &Py<PyDelta>) -> IggyDuration {
+pub fn py_delta_to_iggy_duration(delta1: &Py<PyDelta>) -> PyResult<IggyDuration> {
Python::attach(|py| {
let delta = delta1.bind(py);
- let seconds = (delta.get_days() * 60 * 60 * 24 + delta.get_seconds()) as u64;
+ let total_seconds = delta.get_days() * 60 * 60 * 24 + delta.get_seconds();
Review Comment:
`delta.get_days() * 60 * 60 * 24 + delta.get_seconds()` is all i32 here (pyo3's `get_days`/`get_seconds` both return `i32`). for `|days| >= 24856` (~68yr) the multiply overflows - release wheels wrap silently, debug builds panic. the nasty part is it defeats the negative-duration guard this PR just added right below: a large negative `timedelta` (easy to get from date arithmetic) wraps to a positive `total_seconds`, slips past the `if total_seconds < 0` check, and a positive expiry gets persisted from a negative input. one-line fix: `let total_seconds = i64::from(delta.get_days()) * 86_400 + i64::from(delta.get_seconds());` then range-check before the `as u64`.
##########
foreign/python/src/topic.rs:
##########
@@ -15,9 +15,133 @@
// specific language governing permissions and limitations
// under the License.
-use iggy::prelude::{Topic as RustTopic, TopicDetails as RustTopicDetails};
+use std::time::Duration;
+
+use iggy::prelude::{
+ IggyByteSize, IggyExpiry as RustIggyExpiry, MaxTopicSize as RustMaxTopicSize,
+ Partition as RustPartition, Topic as RustTopic, TopicDetails as RustTopicDetails,
+};
+use pyo3::exceptions::PyValueError;
use pyo3::prelude::*;
-use pyo3_stub_gen::derive::{gen_stub_pyclass, gen_stub_pymethods};
+use pyo3::types::PyDelta;
+use pyo3_stub_gen::derive::{gen_stub_pyclass, gen_stub_pyclass_complex_enum, gen_stub_pymethods};
+
+use crate::consumer::py_delta_to_iggy_duration;
+
+/// The expiry of the messages in a topic.
+#[gen_stub_pyclass_complex_enum]
+#[pyclass]
+pub enum IggyExpiry {
+ /// Use the message expiry configured on the server for this topic,
+ /// rather than an explicit value set by the client.
+ ServerDefault(),
+ /// Expire messages this long after they are appended to the topic.
+ ///
+ /// `duration` must be greater than zero: a zero-length `timedelta` is
+ /// indistinguishable on the wire from `ServerDefault` and is treated as
+ /// such by the server. A negative `timedelta` raises `ValueError` when
+ /// this value is passed to `create_topic`/`update_topic`. The upper
+ /// bound is whatever a `datetime.timedelta` can represent that also fits
+ /// in a `u64` microsecond count (about 584,942 years); in practice the
Review Comment:
this bound isn't accurate today - the timedelta conversion in `py_delta_to_iggy_duration` overflows i32 at ~68 years, so the real ceiling is far below what's documented. the figure is also a typo: u64 microseconds is ~584,542 years, not 584,942. once the overflow is widened to i64 the ~584,542y bound becomes real and this doc is correct.
##########
foreign/python/src/topic.rs:
##########
@@ -15,9 +15,133 @@
// specific language governing permissions and limitations
// under the License.
-use iggy::prelude::{Topic as RustTopic, TopicDetails as RustTopicDetails};
+use std::time::Duration;
+
+use iggy::prelude::{
+ IggyByteSize, IggyExpiry as RustIggyExpiry, MaxTopicSize as RustMaxTopicSize,
+ Partition as RustPartition, Topic as RustTopic, TopicDetails as RustTopicDetails,
+};
+use pyo3::exceptions::PyValueError;
use pyo3::prelude::*;
-use pyo3_stub_gen::derive::{gen_stub_pyclass, gen_stub_pymethods};
+use pyo3::types::PyDelta;
+use pyo3_stub_gen::derive::{gen_stub_pyclass, gen_stub_pyclass_complex_enum, gen_stub_pymethods};
+
+use crate::consumer::py_delta_to_iggy_duration;
+
+/// The expiry of the messages in a topic.
+#[gen_stub_pyclass_complex_enum]
+#[pyclass]
+pub enum IggyExpiry {
+ /// Use the message expiry configured on the server for this topic,
+ /// rather than an explicit value set by the client.
+ ServerDefault(),
+ /// Expire messages this long after they are appended to the topic.
+ ///
+ /// `duration` must be greater than zero: a zero-length `timedelta` is
+ /// indistinguishable on the wire from `ServerDefault` and is treated as
+ /// such by the server. A negative `timedelta` raises `ValueError` when
+ /// this value is passed to `create_topic`/`update_topic`. The upper
+ /// bound is whatever a `datetime.timedelta` can represent that also fits
+ /// in a `u64` microsecond count (about 584,942 years); in practice the
+ /// server-configured maximum is reached long before that.
+ ExpireDuration { duration: Py<PyDelta> },
+ /// Retain messages indefinitely; they never expire.
+ NeverExpire(),
+}
+
+impl From<RustIggyExpiry> for IggyExpiry {
+ fn from(expiry: RustIggyExpiry) -> Self {
+ match expiry {
+ RustIggyExpiry::ServerDefault => IggyExpiry::ServerDefault(),
+ RustIggyExpiry::ExpireDuration(duration) => IggyExpiry::ExpireDuration {
+ duration: iggy_duration_to_py_delta(duration.get_duration()),
+ },
+ RustIggyExpiry::NeverExpire => IggyExpiry::NeverExpire(),
+ }
+ }
+}
+
+impl TryFrom<&IggyExpiry> for RustIggyExpiry {
+ type Error = PyErr;
+
+ fn try_from(expiry: &IggyExpiry) -> PyResult<Self> {
+ Ok(match expiry {
+ IggyExpiry::ServerDefault() => RustIggyExpiry::ServerDefault,
+ IggyExpiry::ExpireDuration { duration } => {
+ RustIggyExpiry::ExpireDuration(py_delta_to_iggy_duration(duration)?)
+ }
+ IggyExpiry::NeverExpire() => RustIggyExpiry::NeverExpire,
+ })
+ }
+}
+
+fn iggy_duration_to_py_delta(duration: Duration) -> Py<PyDelta> {
+ let days = duration.as_secs() / 86_400;
+ let secs_of_day = duration.as_secs() % 86_400;
+ Python::attach(|py| {
+ PyDelta::new(
+ py,
+ days as i32,
+ secs_of_day as i32,
+ duration.subsec_micros() as i32,
+ true,
+ )
+ .expect("topic message expiry duration fits within timedelta bounds")
+ .unbind()
+ })
+}
+
+/// The maximum size of a topic.
+#[gen_stub_pyclass_complex_enum]
+#[pyclass]
+pub enum MaxTopicSize {
+ /// Use the maximum topic size configured on the server, rather than an
+ /// explicit value set by the client.
+ ServerDefault(),
+ /// Cap the topic at this many bytes; as the topic approaches this size,
+ /// the server deletes the oldest sealed segments to make room for new
+ /// messages.
+ ///
+ /// `bytes` must be greater than zero and less than the maximum value of
+ /// an unsigned 64-bit integer: those two values are reserved on the wire
+ /// for `ServerDefault` and `Unlimited` respectively, so a `Custom` size
+ /// at either boundary raises `ValueError` when passed to
+ /// `create_topic`/`update_topic`.
+ Custom { bytes: u64 },
+ /// Do not cap the topic size; it may grow without bound.
+ Unlimited(),
+}
+
+impl From<RustMaxTopicSize> for MaxTopicSize {
+ fn from(max_size: RustMaxTopicSize) -> Self {
+ match max_size {
+ RustMaxTopicSize::ServerDefault => MaxTopicSize::ServerDefault(),
+ RustMaxTopicSize::Custom(size) => MaxTopicSize::Custom {
+ bytes: size.as_bytes_u64(),
+ },
+ RustMaxTopicSize::Unlimited => MaxTopicSize::Unlimited(),
+ }
+ }
+}
+
+impl TryFrom<&MaxTopicSize> for RustMaxTopicSize {
+ type Error = PyErr;
+
+ fn try_from(max_size: &MaxTopicSize) -> PyResult<Self> {
+ Ok(match max_size {
+ MaxTopicSize::ServerDefault() => RustMaxTopicSize::ServerDefault,
+ MaxTopicSize::Custom { bytes } => {
+ if *bytes == 0 || *bytes == u64::MAX {
Review Comment:
small consistency thing: `MaxTopicSize.Custom(0)` (and `u64::MAX`) reject with `ValueError` here, but the sibling sentinel `IggyExpiry.ExpireDuration(timedelta(0))` silently coerces to server-default instead of rejecting. both are documented so it's not a bug, just worth picking one policy across the two - either both reject the reserved value or both coerce it.
##########
foreign/python/src/client.rs:
##########
@@ -187,22 +190,28 @@ impl IggyClient {
#[gen_stub(override_type(type_repr = "builtins.int | None"))] replication_factor: Option<
u8,
>,
- #[gen_stub(override_type(type_repr = "datetime.timedelta | None", imports=("datetime")))]
- message_expiry: Option<Py<PyDelta>>,
- #[gen_stub(override_type(type_repr = "builtins.int | None"))] max_topic_size: Option<u64>,
+ #[gen_stub(override_type(type_repr = "IggyExpiry | None"))] message_expiry: Option<
Review Comment:
note this is a breaking change for `create_topic.message_expiry` - it went from `timedelta | None` to `IggyExpiry | None`, and that param shipped in the released 0.8.0, so existing callers passing a `timedelta` now hit a `TypeError`. the `max_topic_size` retype and all of `update_topic` are post-0.8.0 so those break nothing released. worth calling out the `message_expiry` break, at least in the PR descr.
##########
foreign/python/src/client.rs:
##########
@@ -187,22 +190,28 @@ impl IggyClient {
#[gen_stub(override_type(type_repr = "builtins.int | None"))] replication_factor: Option<
u8,
>,
- #[gen_stub(override_type(type_repr = "datetime.timedelta | None", imports=("datetime")))]
- message_expiry: Option<Py<PyDelta>>,
- #[gen_stub(override_type(type_repr = "builtins.int | None"))] max_topic_size: Option<u64>,
+ #[gen_stub(override_type(type_repr = "IggyExpiry | None"))] message_expiry: Option<
+ &IggyExpiry,
+ >,
+ #[gen_stub(override_type(type_repr = "MaxTopicSize | None"))] max_topic_size: Option<
+ &MaxTopicSize,
+ >,
) -> PyResult<Bound<'a, PyAny>> {
let compression_algorithm = match compression_algorithm {
Some(algo) => CompressionAlgorithm::from_str(&algo)
.map_err(|e| PyErr::new::<pyo3::exceptions::PyRuntimeError, _>(e.to_string()))?,
None => CompressionAlgorithm::default(),
};
- let expiry = match message_expiry {
- Some(delta) => IggyExpiry::ExpireDuration(py_delta_to_iggy_duration(&delta)),
- None => IggyExpiry::ServerDefault,
- };
+ let expiry = message_expiry
Review Comment:
this expiry + max_size resolution block is duplicated byte-for-byte in `update_topic`. could pull it into a small helper (or two) to dedup, no behavior change.
##########
foreign/python/src/client.rs:
##########
@@ -169,7 +170,9 @@ impl IggyClient {
}
/// Creates a new topic with the given parameters.
Review Comment:
minor: `create_topic` documents its return and raises as free prose while `update_topic` below uses the structured Args/Returns/Raises form. worth aligning the two.
##########
foreign/python/tests/test_topic.py:
##########
@@ -895,7 +965,7 @@ async def test_update_topic_invalid_replication_factor(
@pytest.mark.asyncio
@pytest.mark.parametrize(
- ("max_topic_size", "expected_exception"),
+ ("max_topic_size_bytes", "expected_exception"),
[
(-1, OverflowError),
(2e64, TypeError),
Review Comment:
the create_topic version of this test also covers `(0, ValueError)` and `(2**64-1, ValueError)` for the reserved sentinels, but they're missing here even though update goes through the same `try_from`. there's also no update_topic test for a negative `ExpireDuration` like create has. worth mirroring the create cases.
##########
foreign/python/src/topic.rs:
##########
@@ -100,15 +260,102 @@ impl TopicDetails {
self.inner.partitions_count
}
+ /// The timestamp when the topic was created, in microseconds.
+ #[getter]
+ pub fn created_at(&self) -> u64 {
+ self.inner.created_at.as_micros()
+ }
+
+ /// The total size of the topic in bytes.
+ #[getter]
+ pub fn size(&self) -> u64 {
+ self.inner.size.as_bytes_u64()
+ }
+
+ /// The expiry of the messages in the topic.
+ #[getter]
+ pub fn message_expiry(&self) -> IggyExpiry {
+ self.inner.message_expiry.into()
+ }
+
/// Compression algorithm for the topic.
#[getter]
pub fn compression_algorithm(&self) -> String {
self.inner.compression_algorithm.to_string()
}
+ /// The maximum size of the topic.
+ #[getter]
+ pub fn max_topic_size(&self) -> MaxTopicSize {
+ self.inner.max_topic_size.into()
+ }
+
/// Replication factor for the topic.
#[getter]
pub fn replication_factor(&self) -> u8 {
self.inner.replication_factor
}
+
+ /// The collection of partitions in the topic.
+ #[getter]
+ pub fn partitions(&self) -> Vec<Partition> {
Review Comment:
this getter clones the whole partition vec and builds fresh python objects on every access, so `topic.partitions` looks like a cheap attribute but re-materializes each time (and the returned objects don't have stable identity). it's a cold metadata path so not a perf concern, but worth a doc note or exposing it as a method so callers don't read it repeatedly in a loop.
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(python): expose remaining Topic/TopicDetails fields and partitions [iggy]
Posted by "mattp5657 (via GitHub)" <gi...@apache.org>.
mattp5657 commented on code in PR #3623:
URL: https://github.com/apache/iggy/pull/3623#discussion_r3591271619
##########
foreign/python/src/client.rs:
##########
@@ -187,22 +190,28 @@ impl IggyClient {
#[gen_stub(override_type(type_repr = "builtins.int | None"))] replication_factor: Option<
u8,
>,
- #[gen_stub(override_type(type_repr = "datetime.timedelta | None", imports=("datetime")))]
- message_expiry: Option<Py<PyDelta>>,
- #[gen_stub(override_type(type_repr = "builtins.int | None"))] max_topic_size: Option<u64>,
+ #[gen_stub(override_type(type_repr = "IggyExpiry | None"))] message_expiry: Option<
Review Comment:
Sorry, I left a comment but hadn't updated the PR description yet. I know it's sometimes easier to follow a PR when the description reflects the original push and comments show how the discussion evolved, but I should have been clearer here, apologies.
I see three options:
1. Keep the current change. I think it's the most robust approach, but I understand it's a breaking change for `message_expiry`.
2. Revert to the original types used to init the server settings (backwards compatible).
3. Accept both, support the old type alongside the new enum.
My main concern is whether we should preserve backwards compatibility here and add test coverage for it, but I wanted to get your thoughts before going down that path.
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(python): expose remaining Topic/TopicDetails fields and partitions [iggy]
Posted by "slbotbm (via GitHub)" <gi...@apache.org>.
slbotbm commented on code in PR #3623:
URL: https://github.com/apache/iggy/pull/3623#discussion_r3565879423
##########
foreign/python/src/topic.rs:
##########
@@ -15,9 +15,120 @@
// specific language governing permissions and limitations
// under the License.
-use iggy::prelude::{Topic as RustTopic, TopicDetails as RustTopicDetails};
+use std::time::Duration;
+
+use iggy::prelude::{
+ IggyByteSize, IggyExpiry as RustIggyExpiry, MaxTopicSize as RustMaxTopicSize,
+ Partition as RustPartition, Topic as RustTopic, TopicDetails as RustTopicDetails,
+};
use pyo3::prelude::*;
-use pyo3_stub_gen::derive::{gen_stub_pyclass, gen_stub_pymethods};
+use pyo3::types::PyDelta;
+use pyo3_stub_gen::derive::{gen_stub_pyclass, gen_stub_pyclass_complex_enum, gen_stub_pymethods};
+
+use crate::consumer::py_delta_to_iggy_duration;
+
+/// The expiry of the messages in a topic.
+#[gen_stub_pyclass_complex_enum]
+#[pyclass]
+pub enum IggyExpiry {
+ /// Use the message expiry configured on the server for this topic,
+ /// rather than an explicit value set by the client.
+ ServerDefault(),
+ /// Expire messages this long after they are appended to the topic.
+ ///
+ /// `duration` must be greater than zero: a zero-length `timedelta` is
+ /// indistinguishable on the wire from `ServerDefault` and is treated as
+ /// such by the server. The upper bound is whatever a `datetime.timedelta`
+ /// can represent that also fits in a `u64` microsecond count (about
+ /// 584,942 years); in practice the server-configured maximum is reached
+ /// long before that.
+ ExpireDuration { duration: Py<PyDelta> },
+ /// Retain messages indefinitely; they never expire.
+ NeverExpire(),
+}
+
+impl From<RustIggyExpiry> for IggyExpiry {
+ fn from(expiry: RustIggyExpiry) -> Self {
+ match expiry {
+ RustIggyExpiry::ServerDefault => IggyExpiry::ServerDefault(),
+ RustIggyExpiry::ExpireDuration(duration) => IggyExpiry::ExpireDuration {
+ duration: iggy_duration_to_py_delta(duration.get_duration()),
+ },
+ RustIggyExpiry::NeverExpire => IggyExpiry::NeverExpire(),
+ }
+ }
+}
+
+impl From<&IggyExpiry> for RustIggyExpiry {
+ fn from(expiry: &IggyExpiry) -> Self {
+ match expiry {
+ IggyExpiry::ServerDefault() => RustIggyExpiry::ServerDefault,
+ IggyExpiry::ExpireDuration { duration } => {
+ RustIggyExpiry::ExpireDuration(py_delta_to_iggy_duration(duration))
Review Comment:
This also accepts negative duration, which should be rejected as PyValueError
##########
foreign/python/src/topic.rs:
##########
@@ -15,9 +15,120 @@
// specific language governing permissions and limitations
// under the License.
-use iggy::prelude::{Topic as RustTopic, TopicDetails as RustTopicDetails};
+use std::time::Duration;
+
+use iggy::prelude::{
+ IggyByteSize, IggyExpiry as RustIggyExpiry, MaxTopicSize as RustMaxTopicSize,
+ Partition as RustPartition, Topic as RustTopic, TopicDetails as RustTopicDetails,
+};
use pyo3::prelude::*;
-use pyo3_stub_gen::derive::{gen_stub_pyclass, gen_stub_pymethods};
+use pyo3::types::PyDelta;
+use pyo3_stub_gen::derive::{gen_stub_pyclass, gen_stub_pyclass_complex_enum, gen_stub_pymethods};
+
+use crate::consumer::py_delta_to_iggy_duration;
+
+/// The expiry of the messages in a topic.
+#[gen_stub_pyclass_complex_enum]
+#[pyclass]
+pub enum IggyExpiry {
+ /// Use the message expiry configured on the server for this topic,
+ /// rather than an explicit value set by the client.
+ ServerDefault(),
+ /// Expire messages this long after they are appended to the topic.
+ ///
+ /// `duration` must be greater than zero: a zero-length `timedelta` is
+ /// indistinguishable on the wire from `ServerDefault` and is treated as
+ /// such by the server. The upper bound is whatever a `datetime.timedelta`
+ /// can represent that also fits in a `u64` microsecond count (about
+ /// 584,942 years); in practice the server-configured maximum is reached
+ /// long before that.
+ ExpireDuration { duration: Py<PyDelta> },
+ /// Retain messages indefinitely; they never expire.
+ NeverExpire(),
+}
+
+impl From<RustIggyExpiry> for IggyExpiry {
+ fn from(expiry: RustIggyExpiry) -> Self {
+ match expiry {
+ RustIggyExpiry::ServerDefault => IggyExpiry::ServerDefault(),
+ RustIggyExpiry::ExpireDuration(duration) => IggyExpiry::ExpireDuration {
+ duration: iggy_duration_to_py_delta(duration.get_duration()),
+ },
+ RustIggyExpiry::NeverExpire => IggyExpiry::NeverExpire(),
+ }
+ }
+}
+
+impl From<&IggyExpiry> for RustIggyExpiry {
+ fn from(expiry: &IggyExpiry) -> Self {
+ match expiry {
+ IggyExpiry::ServerDefault() => RustIggyExpiry::ServerDefault,
+ IggyExpiry::ExpireDuration { duration } => {
+ RustIggyExpiry::ExpireDuration(py_delta_to_iggy_duration(duration))
+ }
+ IggyExpiry::NeverExpire() => RustIggyExpiry::NeverExpire,
+ }
+ }
+}
+
+fn iggy_duration_to_py_delta(duration: Duration) -> Py<PyDelta> {
+ let days = duration.as_secs() / 86_400;
+ let secs_of_day = duration.as_secs() % 86_400;
+ Python::attach(|py| {
+ PyDelta::new(
+ py,
+ days as i32,
+ secs_of_day as i32,
+ duration.subsec_micros() as i32,
+ true,
+ )
+ .expect("topic message expiry duration fits within timedelta bounds")
+ .unbind()
+ })
+}
+
+/// The maximum size of a topic.
+#[gen_stub_pyclass_complex_enum]
+#[pyclass]
+pub enum MaxTopicSize {
+ /// Use the maximum topic size configured on the server, rather than an
+ /// explicit value set by the client.
+ ServerDefault(),
+ /// Cap the topic at this many bytes; as the topic approaches this size,
+ /// the server deletes the oldest sealed segments to make room for new
+ /// messages.
+ ///
+ /// `bytes` must be greater than zero and less than `u64::MAX`: those two
Review Comment:
Python callers do not know what u64::MAX means.
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(python): expose remaining Topic/TopicDetails fields and partitions [iggy]
Posted by "slbotbm (via GitHub)" <gi...@apache.org>.
slbotbm commented on PR #3623:
URL: https://github.com/apache/iggy/pull/3623#issuecomment-4925937267
/author
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(python): expose remaining Topic/TopicDetails fields and partitions [iggy]
Posted by "mattp5657 (via GitHub)" <gi...@apache.org>.
mattp5657 commented on PR #3623:
URL: https://github.com/apache/iggy/pull/3623#issuecomment-4953233227
Fixed both boundary-collision issues flagged in review: negative `timedelta` values for `IggyExpiry.ExpireDuration` and `0`/`u64::MAX` values for `MaxTopicSize.Custom` were silently accepted and misinterpreted on the wire (as `ServerDefault`/`Unlimited`) instead of being rejected. Both conversions now raise `PyValueError` for out-of-range input, propagated through `create_topic`/`update_topic`. Updated the affected doc comments/stubs and added a regression test for each invalid case.
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(python): expose remaining Topic/TopicDetails fields and partitions [iggy]
Posted by "mattp5657 (via GitHub)" <gi...@apache.org>.
mattp5657 commented on code in PR #3623:
URL: https://github.com/apache/iggy/pull/3623#discussion_r3691545889
##########
foreign/python/src/client.rs:
##########
@@ -187,22 +190,28 @@ impl IggyClient {
#[gen_stub(override_type(type_repr = "builtins.int | None"))] replication_factor: Option<
u8,
>,
- #[gen_stub(override_type(type_repr = "datetime.timedelta | None", imports=("datetime")))]
- message_expiry: Option<Py<PyDelta>>,
- #[gen_stub(override_type(type_repr = "builtins.int | None"))] max_topic_size: Option<u64>,
+ #[gen_stub(override_type(type_repr = "IggyExpiry | None"))] message_expiry: Option<
Review Comment:
Would you like me to update before merge. Also thanks for all your help here!
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(python): expose remaining Topic/TopicDetails fields and partitions [iggy]
Posted by "mattp5657 (via GitHub)" <gi...@apache.org>.
mattp5657 commented on PR #3623:
URL: https://github.com/apache/iggy/pull/3623#issuecomment-4953234419
/ready
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(python): expose remaining Topic/TopicDetails fields and partitions [iggy]
Posted by "mattp5657 (via GitHub)" <gi...@apache.org>.
mattp5657 commented on PR #3623:
URL: https://github.com/apache/iggy/pull/3623#issuecomment-4940412298
/ready
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(python): expose remaining Topic/TopicDetails fields and partitions [iggy]
Posted by "slbotbm (via GitHub)" <gi...@apache.org>.
slbotbm commented on PR #3623:
URL: https://github.com/apache/iggy/pull/3623#issuecomment-5151816015
/author
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(python): expose remaining Topic/TopicDetails fields and partitions [iggy]
Posted by "mattp5657 (via GitHub)" <gi...@apache.org>.
mattp5657 commented on PR #3623:
URL: https://github.com/apache/iggy/pull/3623#issuecomment-5153197888
Thanks for everyones help with this one! I believe the update to the pyproject and the conflict are resolved now.
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(python): expose remaining Topic/TopicDetails fields and partitions [iggy]
Posted by "mattp5657 (via GitHub)" <gi...@apache.org>.
mattp5657 commented on code in PR #3623:
URL: https://github.com/apache/iggy/pull/3623#discussion_r3591271619
##########
foreign/python/src/client.rs:
##########
@@ -187,22 +190,28 @@ impl IggyClient {
#[gen_stub(override_type(type_repr = "builtins.int | None"))] replication_factor: Option<
u8,
>,
- #[gen_stub(override_type(type_repr = "datetime.timedelta | None", imports=("datetime")))]
- message_expiry: Option<Py<PyDelta>>,
- #[gen_stub(override_type(type_repr = "builtins.int | None"))] max_topic_size: Option<u64>,
+ #[gen_stub(override_type(type_repr = "IggyExpiry | None"))] message_expiry: Option<
Review Comment:
Sorry, I left a comment but hadn't updated the PR description yet. I know it's sometimes easier to follow a PR when the description reflects the original push and comments show how the discussion evolved, but I should have been clearer here — apologies.
I see three options:
1. Keep the current change. I think it's the most robust approach, but I understand it's a breaking change for `message_expiry`.
2. Revert to the original types used to init the server settings (backwards compatible).
3. Accept both — support the old type alongside the new enum.
My main concern is whether we should preserve backwards compatibility here and add test coverage for it, but I wanted to get your thoughts before going down that path.
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(python): expose remaining Topic/TopicDetails fields and partitions [iggy]
Posted by "mattp5657 (via GitHub)" <gi...@apache.org>.
mattp5657 commented on PR #3623:
URL: https://github.com/apache/iggy/pull/3623#issuecomment-4935267391
> You've defined MaxTopicSize and IggyExpiry as enums here, but the sdk utilizes normal python ints and timedeltas. Let's change those functions to accept your enums instead of the python ints and structs.
This was an oversight from me, thanks for the call out :)
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(python): expose remaining Topic/TopicDetails fields and partitions [iggy]
Posted by "mattp5657 (via GitHub)" <gi...@apache.org>.
mattp5657 commented on PR #3623:
URL: https://github.com/apache/iggy/pull/3623#issuecomment-4940409119
I made the outlined changes, my only concern is if we should continue with backwards compatibility and incorporate it into testing. But wanted to get your thoughts before doing so. :)
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(python): expose remaining Topic/TopicDetails fields and partitions [iggy]
Posted by "slbotbm (via GitHub)" <gi...@apache.org>.
slbotbm commented on code in PR #3623:
URL: https://github.com/apache/iggy/pull/3623#discussion_r3541634662
##########
foreign/python/src/topic.rs:
##########
@@ -15,9 +15,79 @@
// specific language governing permissions and limitations
// under the License.
-use iggy::prelude::{Topic as RustTopic, TopicDetails as RustTopicDetails};
+use std::time::Duration;
+
+use iggy::prelude::{
+ IggyExpiry as RustIggyExpiry, MaxTopicSize as RustMaxTopicSize, Partition as RustPartition,
+ Topic as RustTopic, TopicDetails as RustTopicDetails,
+};
use pyo3::prelude::*;
-use pyo3_stub_gen::derive::{gen_stub_pyclass, gen_stub_pymethods};
+use pyo3::types::PyDelta;
+use pyo3_stub_gen::derive::{gen_stub_pyclass, gen_stub_pyclass_complex_enum, gen_stub_pymethods};
+
+/// The expiry of the messages in a topic.
+#[gen_stub_pyclass_complex_enum]
+#[pyclass]
+pub enum IggyExpiry {
+ /// Use the server's default message expiry.
+ ServerDefault(),
+ /// Messages expire after this duration.
+ ExpireDuration { duration: Py<PyDelta> },
+ /// Messages never expire.
+ NeverExpire(),
+}
+
+impl From<RustIggyExpiry> for IggyExpiry {
+ fn from(expiry: RustIggyExpiry) -> Self {
+ match expiry {
+ RustIggyExpiry::ServerDefault => IggyExpiry::ServerDefault(),
+ RustIggyExpiry::ExpireDuration(duration) => IggyExpiry::ExpireDuration {
+ duration: iggy_duration_to_py_delta(duration.get_duration()),
+ },
+ RustIggyExpiry::NeverExpire => IggyExpiry::NeverExpire(),
+ }
+ }
+}
+
+fn iggy_duration_to_py_delta(duration: Duration) -> Py<PyDelta> {
+ let days = duration.as_secs() / 86_400;
+ let secs_of_day = duration.as_secs() % 86_400;
+ Python::attach(|py| {
+ PyDelta::new(
+ py,
+ days as i32,
+ secs_of_day as i32,
+ duration.subsec_micros() as i32,
+ true,
+ )
+ .expect("topic message expiry duration fits within timedelta bounds")
+ .unbind()
+ })
+}
+
+/// The maximum size of a topic.
+#[gen_stub_pyclass_complex_enum]
+#[pyclass]
+pub enum MaxTopicSize {
+ /// Use the server's default max size.
+ ServerDefault(),
+ /// The topic is limited to this many bytes.
+ Custom { bytes: u64 },
+ /// The topic has no maximum size.
+ Unlimited(),
+}
Review Comment:
Same here. Make the docs explicit. We are doing this since the user cannot trace what exactly these mean without looking at rust sdk code.
##########
foreign/python/src/topic.rs:
##########
@@ -15,9 +15,79 @@
// specific language governing permissions and limitations
// under the License.
-use iggy::prelude::{Topic as RustTopic, TopicDetails as RustTopicDetails};
+use std::time::Duration;
+
+use iggy::prelude::{
+ IggyExpiry as RustIggyExpiry, MaxTopicSize as RustMaxTopicSize, Partition as RustPartition,
+ Topic as RustTopic, TopicDetails as RustTopicDetails,
+};
use pyo3::prelude::*;
-use pyo3_stub_gen::derive::{gen_stub_pyclass, gen_stub_pymethods};
+use pyo3::types::PyDelta;
+use pyo3_stub_gen::derive::{gen_stub_pyclass, gen_stub_pyclass_complex_enum, gen_stub_pymethods};
+
+/// The expiry of the messages in a topic.
+#[gen_stub_pyclass_complex_enum]
+#[pyclass]
+pub enum IggyExpiry {
+ /// Use the server's default message expiry.
+ ServerDefault(),
+ /// Messages expire after this duration.
+ ExpireDuration { duration: Py<PyDelta> },
+ /// Messages never expire.
+ NeverExpire(),
Review Comment:
Please also write exactly what does server default and never expire mean. Also, range of values that expireduration accepts
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(python): expose remaining Topic/TopicDetails fields and partitions [iggy]
Posted by "slbotbm (via GitHub)" <gi...@apache.org>.
slbotbm commented on code in PR #3623:
URL: https://github.com/apache/iggy/pull/3623#discussion_r3655894628
##########
foreign/python/src/client.rs:
##########
@@ -187,22 +190,28 @@ impl IggyClient {
#[gen_stub(override_type(type_repr = "builtins.int | None"))] replication_factor: Option<
u8,
>,
- #[gen_stub(override_type(type_repr = "datetime.timedelta | None", imports=("datetime")))]
- message_expiry: Option<Py<PyDelta>>,
- #[gen_stub(override_type(type_repr = "builtins.int | None"))] max_topic_size: Option<u64>,
+ #[gen_stub(override_type(type_repr = "IggyExpiry | None"))] message_expiry: Option<
Review Comment:
before reaching 1.0.0, I do not think we guarantee backwards compatibility. I think that writing the message about the breaking changes is enough. You could also additionally update the version of the sdk from `0.8.1.dev3` to `0.8.1.dev4` in pyproject.toml.
Other than that, this looks mergeable.
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(python): expose remaining Topic/TopicDetails fields and partitions [iggy]
Posted by "mattp5657 (via GitHub)" <gi...@apache.org>.
mattp5657 commented on PR #3623:
URL: https://github.com/apache/iggy/pull/3623#issuecomment-4925177270
> ## Which issue does this PR address?
> Closes #3577
>
> ## Rationale
> Continuation of #3572. That PR exposed `Topic`/`TopicDetails` basics but left several fields returned by the server unavailable in Python, so callers couldn't inspect a topic's expiry, size, max size, or its partitions without another SDK.
>
> ## What changed?
> `Topic` and `TopicDetails` previously exposed only `id`, `name`, `partitions_count`, `compression_algorithm`, and `replication_factor`, leaving `created_at`, `size`, `message_expiry`, `max_topic_size`, and per-partition data unavailable in Python.
>
> Both classes now expose `created_at`, `size`, `message_expiry` (new `IggyExpiry` enum), and `max_topic_size` (new `MaxTopicSize` enum). `TopicDetails` also gains `partitions`, a list of new `Partition` objects. `Partition` (`core/common`) now derives `Clone` to support building that list.
>
> ## Local Execution
> * Passed
> * Pre-commit hooks ran
>
> ## AI Usage
> Claude was used to help generate and review this PR.
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(python): expose remaining Topic/TopicDetails fields and partitions [iggy]
Posted by "mattp5657 (via GitHub)" <gi...@apache.org>.
mattp5657 commented on PR #3623:
URL: https://github.com/apache/iggy/pull/3623#issuecomment-5153198700
/ready
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(python): expose remaining Topic/TopicDetails fields and partitions [iggy]
Posted by "slbotbm (via GitHub)" <gi...@apache.org>.
slbotbm commented on code in PR #3623:
URL: https://github.com/apache/iggy/pull/3623#discussion_r3695780974
##########
foreign/python/src/client.rs:
##########
@@ -187,22 +190,28 @@ impl IggyClient {
#[gen_stub(override_type(type_repr = "builtins.int | None"))] replication_factor: Option<
u8,
>,
- #[gen_stub(override_type(type_repr = "datetime.timedelta | None", imports=("datetime")))]
- message_expiry: Option<Py<PyDelta>>,
- #[gen_stub(override_type(type_repr = "builtins.int | None"))] max_topic_size: Option<u64>,
+ #[gen_stub(override_type(type_repr = "IggyExpiry | None"))] message_expiry: Option<
Review Comment:
@mattp5657 Sure, please update the version before it gets merged.
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org