Skip to content

Tomandersen/transaction options - #3664

Merged
tom-andersen merged 29 commits into
masterfrom
tomandersen/transactionOptions
Apr 27, 2022
Merged

Tomandersen/transaction options#3664
tom-andersen merged 29 commits into
masterfrom
tomandersen/transactionOptions

Conversation

@tom-andersen

@tom-andersen tom-andersen commented Apr 19, 2022 •
edited by wu-hui
Loading

Copy link
Copy Markdown
Contributor

Add TransactionOptions to allow control of maximum attempts to commit before transaction fails.

Fix: #2877

eldhosembabu and others added 23 commits February 23, 2022 16:21
@eldhosembabu
- Changes to protoc version to make it working for m1 macs.
246a067
@eldhosembabu
- Changes to protoc version to make it working for m1 macs.
f7873ee
@eldhosembabu
- Changes to dependency versions to make it working for m1 macs.
5a31b19
@eldhosembabu
Merge remote-tracking branch 'origin/master' into m1-mac-fixes
1208d9d
@eldhosembabu
Merge remote-tracking branch 'origin/master' into m1-mac-fixes
97518d5
@eldhosembabu
- Reverting the robolectric version change
b8a85ef
@eldhosembabu
Revert "- Changes to dependency versions to make it working for m1 ma…
80da16e 
…cs."

This reverts commit 5a31b19
@eldhosembabu
- Updating robolectric versions to 4.7+ to get support for m1 macs.
cca600e 
See: robolectric/robolectric#6311
@eldhosembabu
Reverting some changes to check
090773a
@eldhosembabu
Merge remote-tracking branch 'origin/master' into m1-mac-fixes
73dd784
@eldhosembabu
Merge remote-tracking branch 'origin/master' into m1-mac-fixes
09e9664
@eldhosembabu
Merge remote-tracking branch 'origin/master' into m1-mac-fixes
61c4819
@eldhosembabu
Merge remote-tracking branch 'origin/master' into m1-mac-fixes
fcb9e05
@eldhosembabu
Merge remote-tracking branch 'origin/master' into m1-mac-fixes
c3132b1
@eldhosembabu
Merge remote-tracking branch 'origin/master' into m1-mac-fixes
3191148
@eldhosembabu
Merge remote-tracking branch 'origin/master' into m1-mac-fixes
c64e321
@eldhosembabu
Merge remote-tracking branch 'origin/master' into m1-mac-fixes
1fc153b
@eldhosembabu
Merge remote-tracking branch 'origin/master' into m1-mac-fixes
e70532f
@eldhosembabu
Merge remote-tracking branch 'origin/master' into m1-mac-fixes
b155fe7
@tom-andersen
Merge remote-tracking branch 'origin/master' into m1-mac-fixes
5119f6b
@tom-andersen
Merge remote-tracking branch 'origin/master' into m1-mac-fixes
5c5dcdb
@tom-andersen
Merge remote-tracking branch 'origin/master' into m1-mac-fixes
af391ae
@tom-andersen
Add TransactionOptions to allow control over number of attempts to co…
a93bfce 
…mmit, before transaction fails.
@tom-andersen
tom-andersen requested a review from wu-hui April 19, 2022 23:14
@google-oss-bot

google-oss-bot commented Apr 19, 2022

Copy link
Copy Markdown
Collaborator

The public api surface has changed for the subproject firebase-firestore:
error: Added method com.google.firebase.firestore.FirebaseFirestore.runTransaction(com.google.firebase.firestore.TransactionOptions,com.google.firebase.firestore.Transaction.Function) [AddedMethod]
error: Added class com.google.firebase.firestore.TransactionOptions [AddedClass]

Please update the api.txt files for the subprojects being affected by this change by running ./gradlew ${subproject}:generateApiTxtFile. Also perform a major/minor bump accordingly.

@google-oss-bot

google-oss-bot commented Apr 19, 2022 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Coverage Report 1

Affected Products

  • firebase-firestore

    Overall coverage changed from 46.25% (b600e83) to 46.04% (40b73c5) by -0.21%.

    FilenameBase (b600e83)Merge (40b73c5)Diff
    AsyncQueue.java78.61%78.11%-0.50%
    Datastore.java32.14%29.76%-2.38%
    FirebaseFirestore.java37.08%36.87%-0.21%
    FirestoreChannel.java18.10%16.38%-1.72%
    FirestoreClient.java38.52%30.88%-7.64%
    GrpcCallProvider.java69.41%62.35%-7.06%
    IndexBackfiller.java92.19%76.56%-15.63%
    LruGarbageCollector.java93.46%84.11%-9.35%
    SetMutation.java94.44%97.22%+2.78%

Test Logs

Notes

  • Commit (40b73c5) is created by Prow via merging PR base commit (b600e83) and head commit (1d62d46).
  • Run gradle <product>:checkCoverage to produce HTML coverage reports locally. After gradle commands finished, report files can be found under <product-build-dir>/reports/jacoco/.

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/qLDYYy7VUN.html

@tom-andersen
Merge remote-tracking branch 'origin/tomandersen/transactionOptions' …
c6d0361 
…into tomandersen/transactionOptions

# Conflicts:
#	firebase-firestore/src/main/java/com/google/firebase/firestore/TransactionOptions.java
@google-oss-bot

google-oss-bot commented Apr 19, 2022 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Size Report 1

Affected Products

  • firebase-firestore

    TypeBase (b600e83)Merge (40b73c5)Diff
    aar1.25 MB1.26 MB+2.19 kB (+0.2%)
    apk (release)3.38 MB3.38 MB+852 B (+0.0%)

Test Logs

Notes

  • Commit (40b73c5) is created by Prow via merging PR base commit (b600e83) and head commit (1d62d46).

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/dTHDZirEe8.html

@google-oss-bot

google-oss-bot commented Apr 19, 2022

Copy link
Copy Markdown
Collaborator

The public api surface has changed for the subproject firebase-firestore:
error: Added method com.google.firebase.firestore.FirebaseFirestore.runTransaction(com.google.firebase.firestore.TransactionOptions,com.google.firebase.firestore.Transaction.Function) [AddedMethod]
error: Added class com.google.firebase.firestore.TransactionOptions [AddedClass]

Please update the api.txt files for the subprojects being affected by this change by running ./gradlew ${subproject}:generateApiTxtFile. Also perform a major/minor bump accordingly.

@tom-andersen
tom-andersen marked this pull request as ready for review April 19, 2022 23:46
@wu-hui
wu-hui requested a review from markarndt April 20, 2022 00:45
wu-hui
wu-hui requested changes Apr 20, 2022
View reviewed changes

@wu-hui wu-hui left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good, some nits on comments.

Comment thread firebase-firestore/src/main/java/com/google/firebase/firestore/FirebaseFirestore.java Outdated
Comment thread firebase-firestore/src/main/java/com/google/firebase/firestore/FirebaseFirestore.java Outdated
Comment thread firebase-firestore/src/main/java/com/google/firebase/firestore/TransactionOptions.java Outdated
Comment thread firebase-firestore/src/main/java/com/google/firebase/firestore/TransactionOptions.java
Comment thread firebase-firestore/src/main/java/com/google/firebase/firestore/TransactionOptions.java Outdated
Comment thread firebase-firestore/src/main/java/com/google/firebase/firestore/TransactionOptions.java Outdated
Comment thread firebase-firestore/src/main/java/com/google/firebase/firestore/TransactionOptions.java Outdated
@wu-hui wu-hui assigned tom-andersen and unassigned wu-hui Apr 20, 2022
@wu-hui

wu-hui commented Apr 20, 2022

Copy link
Copy Markdown
Contributor

You should also add an entry to CHANGELOG.md.

See here: https://osscs.corp.google.com/firebase-sdk/firebase-android-sdk/+/master:firebase-firestore/CHANGELOG.md;l=5

tom-andersen
tom-andersen commented Apr 21, 2022
View reviewed changes

@tom-andersen tom-andersen left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed comments

Comment thread firebase-firestore/src/main/java/com/google/firebase/firestore/TransactionOptions.java Outdated
Comment thread firebase-firestore/src/main/java/com/google/firebase/firestore/TransactionOptions.java
Comment thread firebase-firestore/src/main/java/com/google/firebase/firestore/TransactionOptions.java Outdated
Comment thread firebase-firestore/src/main/java/com/google/firebase/firestore/TransactionOptions.java Outdated
Comment thread firebase-firestore/src/main/java/com/google/firebase/firestore/TransactionOptions.java Outdated
tom-andersen
tom-andersen commented Apr 21, 2022
View reviewed changes

@tom-andersen tom-andersen left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed changes

@tom-andersen
tom-andersen requested a review from wu-hui April 21, 2022 22:14
wu-hui
wu-hui approved these changes Apr 22, 2022
View reviewed changes

@wu-hui wu-hui left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but please merge it after you get tech writer approval.

@wu-hui wu-hui removed their assignment Apr 22, 2022
@tom-andersen
tom-andersen merged commit 672c6dc into master Apr 27, 2022
@tom-andersen
tom-andersen deleted the tomandersen/transactionOptions branch April 27, 2022 18:00
dconeybe added a commit to firebase/firebase-ios-sdk that referenced this pull request May 25, 2022
@dconeybe
Firestore/CHANGELOG.md: Copied message from firebase/firebase-android…
10e2001 
…-sdk#3664
This was referenced May 25, 2022
Merged
Closed
Merged
@firebase firebase locked and limited conversation to collaborators May 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@wu-hui wu-hui wu-hui approved these changes
@markarndt markarndt Awaiting requested review from markarndt

Assignees

@tom-andersen tom-andersen

@markarndt markarndt

Labels

size/L

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Very long (10 - 30 seconds) transaction failure timeout while offline

5 participants

@tom-andersen @google-oss-bot @wu-hui @markarndt @eldhosembabu