Commit 4367098
authored
fix: Pass
<!--
Thanks for opening a pull request!
-->
<!-- In the case this PR will resolve an issue, please replace
${GITHUB_ISSUE_ID} below with the actual Github issue id. -->
<!-- Closes #${GITHUB_ISSUE_ID} -->
Closes #2558
# Rationale for this change
When removing snapshots with statistics, `RemoveStatisticsUpdate` was
being instantiated with a positional argument, which, as suggested by
@vndv, "violates Pydantic's BaseModel requirement that all fields be
passed as keyword arguments". Shout out to @vndv for catching this 🚀
This caused a `TypeError: BaseModel.__init__() takes 1 positional
argument but 2 were given` when calling
`table.maintenance.expire_snapshots().older_than(...).commit()`.
## Are these changes tested?
Yes, and a new test was added.
Existing tests only tested the `RemoveStatisticsUpdate` directly, but
didnt test the code path through `RemoveSnapshotsUpdate` that triggers
the bug.
Added `test_update_remove_snapshots_with_statistics` to
`test_expire_snapshots.py` to extend coverage for the condition.
## Are there any user-facing changes?
No.
<!-- In the case of user-facing changes, please add the changelog label.
-->snapshot_id as kwarg to RemoveStatisticsUpdate (#2694)1 parent bb41a6d commit 4367098
File tree
2 files changed
+38
-1
lines changed- pyiceberg/table/update
- tests/table
2 files changed
+38
-1
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
528 | 528 | | |
529 | 529 | | |
530 | 530 | | |
531 | | - | |
| 531 | + | |
532 | 532 | | |
533 | 533 | | |
534 | 534 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
23 | 23 | | |
24 | 24 | | |
25 | 25 | | |
| 26 | + | |
26 | 27 | | |
27 | 28 | | |
28 | 29 | | |
| |||
280 | 281 | | |
281 | 282 | | |
282 | 283 | | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
| 303 | + | |
| 304 | + | |
| 305 | + | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
| 312 | + | |
| 313 | + | |
| 314 | + | |
| 315 | + | |
| 316 | + | |
| 317 | + | |
| 318 | + | |
| 319 | + | |
0 commit comments