-
Notifications
You must be signed in to change notification settings - Fork 344
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! |
liurenjie1024
left a comment
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
transactionmodule to provide a basic example of theFastAppendActionusage.Other actions are not included at this time.
it looks like this
Are these changes tested?
N/A