feat(transaction): Implement TransactionAction for FastAppendAction #1448
Conversation
…ducer, remove add_parquet_files
| @@ -127,7 +127,7 @@ impl ManifestWriter { | |||
| pub(crate) fn new( | |||
| output: OutputFile, | |||
| snapshot_id: Option<i64>, | |||
| key_metadata: Vec<u8>, | |||
| key_metadata: Option<Vec<u8>>, | |||
There was a problem hiding this comment.
This is an API change, would appreciate if we could call it out at PR description and release notes.
There was a problem hiding this comment.
Sorry about that! I've updated the PR description.
cc @liurenjie1024 probably related to the release process: Is there a "staging release notes" that we can start putting changes like this into as breaking changes?
There was a problem hiding this comment.
I'm thinking about adding a breaking label to pr so that it standout when we generate release notes.
|
|
||
| /// Finished building the action and apply it to the transaction. | ||
| pub async fn apply(self) -> Result<Transaction> { |
There was a problem hiding this comment.
This is an API change, would appreciate PR description and release notes, even provide a small example for no-op migration.
There was a problem hiding this comment.
Updated the PR description accordingly as well
Which issue does this PR close?
Related Issues:
What changes are included in this PR?
BREAKING CHANGES:
ManifestWriternow takeskey_metadata: Option<Vec<u8>>instead ofkey_metadata: Vec<u8>FastAppendAction::applywithcommitas an implementation ofTransactionActionGeneral:
TransactionActionforFastAppendActionSnapshotProduceActiontoSnapshotProducerSnapshotProduceActionan stateless helper class that helps with fast_append::commit (as discussed here)Are these changes tested?
Added unit tests