feat(cli): deprecate version command in favor of --version flag#3206
Conversation
rambleraptor
left a comment
There was a problem hiding this comment.
This looks great to me. Thanks for this!
geruh
left a comment
There was a problem hiding this comment.
Thanks for iterating on the @chrisqiqiu! Just one small comment everything else looks good to me.
rambleraptor
left a comment
There was a problem hiding this comment.
Looks good to me! Thanks for doing this!
|
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that's incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions. |
|
Hi @geruh , when you have a moment, could you pls take another look to see if the changes addressed your feedback? Thanks! |
|
Thanks for the PR @chrisqiqiu and thank you for the review @rambleraptor @geruh |
Closes #3160
Rationale for this change
The
pyiceberg versionsubcommand is non-standard. Most CLI tools use--versioninstead.This PR adds
--versionvia Click's built-inclick.version_option()and deprecates theold
versionsubcommand with a visible warning message, following the project's deprecationconventions.
Note: I chose
removed_in="1.0.0"to match the existing deprecation in the REST catalog(see
pyiceberg/catalog/rest/__init__.py), but happy to adjust if a different version is preferred.Are these changes tested?
Yes. Added two new tests:
test_version_flag— verifiespyiceberg --versionworks correctlytest_version_command_emits_deprecation_warning— verifies the old command still works but emits aDeprecationWarningand prints a visible deprecation notice to stderrUpdated the existing

test_version_does_not_load_catalogto account for the deprecation warning.Are there any user-facing changes?
Yes:
pyiceberg --versionis now availablepyiceberg versionstill works but prints a deprecation warning