Python: Add list-refs CLI command#8570
Python: Add list-refs CLI command#8570amogh-jahagirdar wants to merge 1 commit intoapache:masterfrom
Conversation
amogh-jahagirdar
force-pushed
the
pythoncli-list-refs
branch
from
21c6634 to
e2305e4
Compare
| name = ref_tup[0] | ||
| ref = ref_tup[1] | ||
| if ref.snapshot_ref_type == 'branch': | ||
| min_snapshots_to_keep = str(ref.min_snapshots_to_keep) |
There was a problem hiding this comment.
Still thinking through what's best to surface to a user when the retention properties aren't set. I wonder if the current outputting of "None" will confuse users into thinking somehow there isn't any retention but that's not really the case. During snapshot expiration we fall back to the table level property. Maybe I just load the table property to fill in these values in case it's None.
There was a problem hiding this comment.
I agree that it is best to pull in the table properties, and show the fall back expiration time.
amogh-jahagirdar
force-pushed
the
pythoncli-list-refs
branch
from
e2305e4 to
3fc5919
Compare
Fokko
left a comment
Fokko
left a comment
There was a problem hiding this comment.
Great work @amogh-jahagirdar, some small suggestions
| """List all the refs in the provided table""" | ||
| catalog, output = _catalog_and_output(ctx) | ||
| table = catalog.load_table(identifier) | ||
| refs = table.metadata.refs |
There was a problem hiding this comment.
We try to avoid accessing the metadata directly (that should have been private ..)
There was a problem hiding this comment.
Sure, I added a new refs() API on Table, let me know what you think!
| name = ref_tup[0] | ||
| ref = ref_tup[1] | ||
| if ref.snapshot_ref_type == 'branch': | ||
| min_snapshots_to_keep = str(ref.min_snapshots_to_keep) |
There was a problem hiding this comment.
I agree that it is best to pull in the table properties, and show the fall back expiration time.
amogh-jahagirdar
force-pushed
the
pythoncli-list-refs
branch
7 times, most recently
from
e046aed to
e954c7e
Compare
amogh-jahagirdar
force-pushed
the
pythoncli-list-refs
branch
from
e954c7e to
1372480
Compare
| for name, type, ref_detail in ref_details: | ||
| refs_table.add_row(name, type, ref_detail['max_ref_age_ms'], ref_detail['min_snapshots_to_keep'], ref_detail['max_snapshot_age_ms']) |
There was a problem hiding this comment.
I changed it to putting the ref name in the first column, followed by the type (previously it was the other way around, but it made more sense to me to have the name be first)
| """List all the refs in the provided table""" | ||
| catalog, output = _catalog_and_output(ctx) | ||
| table = catalog.load_table(identifier) | ||
| refs = table.metadata.refs |
There was a problem hiding this comment.
Sure, I added a new refs() API on Table, let me know what you think!
|
Will publish this PR to the new iceberg-python repo https://github.com/apache/iceberg-python, closing this PR |
2 participants
This change adds a list-refs CLI command
