Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Firestore TransactionOptions added, to specify maxAttempts #966

Merged
merged 66 commits into from
Jun 27, 2022
Merged

Firestore TransactionOptions added, to specify maxAttempts #966

merged 66 commits into from
Jun 27, 2022

Conversation

dconeybe
Copy link
Contributor

This is a port of firebase/firebase-ios-sdk#9838 and firebase/firebase-android-sdk#3664, which adds the new TransactionOptions class to Firestore, which can be used to specify the maximum number of attempts to run a transaction before giving up. Previously, this maximum was hardcoded to 5.

dconeybe and others added 19 commits May 6, 2022 15:23
@dconeybe
firestore.cmake: remove the snappy patch and just update the leveldb …
83306fd 
…version
@dconeybe
REVERT ME: Pin firebase-ios-sdk to firebase/firebase-ios-sdk@ece6774
346a4a4
@jonsimantov
Merge branch 'main' into dconeybe/SnappyPatchRevert
e8f77a4
@dconeybe
Merge branch 'main' into dconeybe/SnappyPatchRevert
6557527
@dconeybe
Merge remote-tracking branch 'origin/main' into SnappyPatchRevert
12bc437
@dconeybe
firestore.cmake: updated pinned firebase-ios-sdk to HEAD now that the…
57d03a9 
… PR that adds Snappy support has been merged there
@dconeybe
CMakeLists.txt: set(RE2_BUILD_TESTING OFF CACHE BOOL "")
bdcbbbc
@dconeybe
Patch in #932 to get the C++ SDK to build successfully against HEAD o…
a1d4c3e 
…f the firebase-ios-sdk
@dconeybe
firestore.cmake: pin to the HEAD commit of firebase/firebase-ios-sdk#…
ae41684 
…9838, which adds TransactionOptions
@dconeybe
transaction_options.h: skeleton added
20240d6
@dconeybe
transaction_options_test.cc: add tests for copy and move constructor …
de41aba 
…and assignment operator.
@dconeybe
transaction_options.cc: add ToString() and << std::ostream operator.
4d20243
@dconeybe
transaction_options.h: add == and != operator overloads
3934e9e
@dconeybe
transaction_options.h: add doxygen comments
1240f7e
@dconeybe
transaction_options_test.cc: add test for trivially copyable
0c920be
@dconeybe
transaction_options_test.cc: minor whitespace cleanup
57f9fa4
@dconeybe
transaction_options.cc: use SimpleThrowInvalidArgument instead of SIM…
a030d85 
…PLE_HARD_ASSERT when given an invalid max_attempts
@dconeybe
firestore.h: add RunTransaction() overload that takes TransactionOptions
2608afc
@dconeybe
transaction_test.cc: add test for specifying max_attempts to Transact…
04bb742 
…ionListener.
@dconeybe dconeybe self-assigned this May 25, 2022
@dconeybe
format code
b12fcc8
@dconeybe
release_build_files/readme.md: release notes entry added
1844607
@dconeybe
Merge remote-tracking branch 'origin/main' into SnappyPatchRevert
c9dbc89
@dconeybe
firestore.cmake: update pinned commit to #967, which fixes a problema…
ce9ca4f 
…tic python_setup.cmake bug
@dconeybe
firestore.cmake: update pinned commit to firebase/firebase-ios-sdk#9851…
c834bc6 
…, which fixes a problematic usage of designated initialization
@dconeybe
cherry-pick bundle_test.cc: Disable LoadBundleWithDocumentsAlreadyPul…
b39452a 
…ledFromBackend (#972)
@dconeybe
bundle_test.cc: disable ALL of the tests
f8e38b1
@dconeybe
DO NOT MERGE: Android/firebase_dependencies.gradle: use snapshot vers…
0fe44d9 
…ion of firestore
@dconeybe
DO NOT MERGE: local build optimizations to build.gradle and gradle.pr…
809a98f 
…operties and settings.gradle
@github-actions
Copy link

github-actions bot commented Jun 24, 2022 •
edited

❌  Integration test FAILED

Requested by @dconeybe on commit 252ab6c
Last updated: Mon Jun 27 10:24 PDT 2022
View integration test log & download artifacts

Failures Configs
firestore [TEST] [FAILURE] [iOS] [macos] [1/2 ios_device: simulator_target]
(1 failed tests)  NumericTransformsTest.CreateDocumentWithIncrement
[TEST] [FLAKINESS] [Android] [All 3 os] [1/2 android_device: android_target]
(1 failed tests)  CRASH/TIMEOUT

Add flaky tests to go/fpl-cpp-flake-tracker

@github-actions github-actions bot added the tests: failed This PR's integration tests failed. label Jun 24, 2022
@firebase-workflow-trigger firebase-workflow-trigger bot removed the tests: in-progress This PR's integration tests are in progress. label Jun 24, 2022
@github-actions github-actions bot added tests: in-progress This PR's integration tests are in progress. tests: succeeded This PR's integration tests succeeded. and removed tests: failed This PR's integration tests failed. labels Jun 24, 2022
@firebase-workflow-trigger firebase-workflow-trigger bot removed the tests: in-progress This PR's integration tests are in progress. label Jun 24, 2022
@dconeybe
Copy link
Contributor Author

Everything looks good on desktop and iOS (https://github.com/firebase/firebase-cpp-sdk/actions/runs/2553982778).

Need to test Android once this repository's dependency on com.google.firebase:firebase-firestore is upgraded to 24.2.0 or later (BOM version 30.2.0 or later). #1007 is the currently-pending PR that will do this.

@dconeybe
Copy link
Contributor Author

dconeybe commented Jun 25, 2022 •
edited

Android dependencies have now been upgraded: #1007

I've kicked off integration tests to test all platforms, including Android: https://github.com/firebase/firebase-cpp-sdk/runs/7051060951

I've also kicked off the packaging workflow to make sure it won't break anything: https://github.com/firebase/firebase-cpp-sdk/actions/runs/2559321031

@dconeybe dconeybe added the tests-requested: quick Trigger a quick set of integration tests. label Jun 25, 2022
@github-actions github-actions bot added tests: in-progress This PR's integration tests are in progress. tests: succeeded This PR's integration tests succeeded. and removed tests-requested: quick Trigger a quick set of integration tests. tests: succeeded This PR's integration tests succeeded. labels Jun 25, 2022
@firebase-workflow-trigger firebase-workflow-trigger bot removed the tests: in-progress This PR's integration tests are in progress. label Jun 25, 2022
@dconeybe dconeybe marked this pull request as ready for review June 25, 2022 13:15
@dconeybe dconeybe requested a review from wu-hui June 25, 2022 13:16
@dconeybe dconeybe merged commit 252ab6c into main Jun 27, 2022
@dconeybe dconeybe deleted the dconeybe/TransactionOptions branch June 27, 2022 14:15
@github-actions github-actions bot added tests: in-progress This PR's integration tests are in progress. and removed tests: succeeded This PR's integration tests succeeded. labels Jun 27, 2022
@dconeybe dconeybe mentioned this pull request Jun 27, 2022
Merged
@github-actions github-actions bot added the tests: failed This PR's integration tests failed. label Jun 27, 2022
@firebase-workflow-trigger firebase-workflow-trigger bot removed the tests: in-progress This PR's integration tests are in progress. label Jun 27, 2022
@firebase firebase locked and limited conversation to collaborators Jul 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

@tom-andersen tom-andersen Awaiting requested review from tom-andersen

Assignees

@tom-andersen tom-andersen

@dconeybe dconeybe

Labels
api: firestore tests: failed This PR's integration tests failed.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@dconeybe @wu-hui @tom-andersen @jonsimantov