Repository navigation
feat(python): expose snapshot - #4616
debangshu919 wants to merge 8 commits into
Conversation
|
Thanks for the PR. It is labeled Slash commands (own line, regular comment) move it around the queue:
See CONTRIBUTING.md for details. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4616 +/- ##
============================================
- Coverage 90.76% 90.75% -0.01%
Complexity 2966 2966
============================================
Files 1459 1460 +1
Lines 347741 347824 +83
Branches 292547 292547
============================================
+ Hits 315617 315669 +52
- Misses 25507 25538 +31
Partials 6617 6617
🚀 New features to boost your workflow:
|
|
/skill team-review-slim |
There was a problem hiding this comment.
Summary: The Python snapshot() binding mirrors the Rust SDK and its dedupe protects the wire encoder, but it inherits a fixed 30-second transport deadline that large log snapshots can exceed. The method also trims compression but not snapshot_types, and the new dedupe path has no test.
Counts: critical 0, warning 1, nit 2, simplification 0
This review was generated by Claude Code 2.1.284 on deepseek-flash[1m]. Review the output before you act on it.
|
/ready |
slbotbm
left a comment
There was a problem hiding this comment.
Let's add snapshot compression and snapshot types as enums instead of allowing the user to freely enter strings for them.
57c591f to
96c5e4d
Compare
|
Hi @slbotbm thanks for the review! Please let me know if any further adjustments are needed. |
|
/ready |
|
|
||
| let mut parsed_types = Vec::with_capacity(snapshot_types.len()); | ||
| for snapshot_type in snapshot_types { | ||
| let rust_type: SystemSnapshotType = snapshot_type.into(); |
There was a problem hiding this comment.
Let's make this SystemSnapshotType::from(snapshot_type)
| } | ||
|
|
||
| let inner = self.inner.clone(); | ||
| let compression: SnapshotCompression = snapshot_compression.into(); |
There was a problem hiding this comment.
SnapshotCompression::from(snapshot_compression)
| #[cfg(test)] | ||
| mod tests { | ||
| use super::*; | ||
|
|
||
| #[test] |
There was a problem hiding this comment.
these tests are not required
| @pytest.mark.unit | ||
| def test_snapshot_enums(self): | ||
| """Snapshot compression and type enums expose expected variants.""" | ||
| assert SnapshotCompression.Deflated == SnapshotCompression.Deflated | ||
| assert SnapshotCompression.Deflated != SnapshotCompression.Stored | ||
| assert SystemSnapshotType.All == SystemSnapshotType.All |
Which issue does this PR address?
Closes #4611
Rationale
The Python SDK lacked support for taking server diagnostic snapshots, a capability already available in the Rust and C++ SDKs.
What changed?
The Python client had no interface to request server diagnostic snapshots.
IggyClient.snapshot()now captures server snapshots and returns compressed ZIP archive bytes. The method accepts strongly typedSnapshotCompressionandSystemSnapshotTypeenums instead of raw strings, providing compile-time/IDE autocompletion and early type validation. The changes include full PyO3 bindings, regenerated type stubs inapache_iggy.pyi, and comprehensive unit and integration tests.Local Execution
AI Usage
iggy serverwith pytest (test_snapshot.py), rancargo clippyand verified withruff.