-
Notifications
You must be signed in to change notification settings - Fork 281
docs: add Transaction
example
#1436
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
docs: add Transaction
example
#1436
Conversation
433a900
to
17d3e20
Compare
Hi @jdockerty , thanks for this PR! I do think adding more documentation is necessary since iceberg-rs's tx semantics is slightly different from iceberg-java. There is an ongoing effort of standardlizing transaction api and will change the existing usages a little bit, maybe we can wait until that work is finished and add the new doc later: |
@CTTY This sounds reasonable to me, thanks for the heads up 💯 For my understanding, the linked PRs are fundamentally changing the E.g. there are more well-defined actions on the new path, such as an It doesn't look like this affects the |
Yes, the ongoing refactoring effort is for all transaction actions. I'm planning to work on |
Hi @jdockerty , with the closing of #1451, Transaction API semantic should be finalized as of now. Please feel free to resume this effort! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @jdockerty for this pr, looks good to me!
Which issue does this PR close?
N/A, although I can link this to the Write support epic is that is useful?
What changes are included in this PR?
Adding additional documentation to the
transaction
module to provide a basic example of theFastAppendAction
usage.Other actions are not included at this time.
it looks like this
Are these changes tested?
N/A