-
-
Notifications
You must be signed in to change notification settings - Fork 214
feat: Add client-to-server request retry parameter support #1103
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
base: master
Are you sure you want to change the base?
Conversation
|
🚀 Thanks for opening this pull request! |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds two optional parameters ( Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
🔇 Additional comments (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1103 +/- ##
=======================================
Coverage 44.37% 44.37%
=======================================
Files 62 62
Lines 3727 3727
=======================================
Hits 1654 1654
Misses 2073 2073 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Pull request overview
This pull request exposes retry configuration parameters for REST API requests in the Flutter SDK wrapper to match functionality already available in the underlying Dart package (v9.5.0). This allows Flutter developers to configure automatic retry behavior for failed HTTP requests, with separate configurations for read operations (GET, DELETE) versus write operations (POST, PUT).
- Updated dependency constraint to require Dart SDK v9.5.0 or higher
- Added
restRetryIntervalsandrestRetryIntervalsForWritesparameters toParse.initialize() - Both parameters are forwarded to the underlying Dart package without modification
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/flutter/pubspec.yaml | Updates minimum Dart SDK version from 9.4.2 to 9.5.0 to support retry configuration |
| packages/flutter/lib/parse_server_sdk_flutter.dart | Adds restRetryIntervals and restRetryIntervalsForWrites parameters to Parse.initialize() method and forwards them to the underlying Dart SDK |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/flutter/lib/parse_server_sdk_flutter.dart (1)
69-71: New retry interval parameters are correctly threaded through; consider documenting themThe additions of
restRetryIntervalsandrestRetryIntervalsForWritestoParse.initializeand their direct forwarding tosuper.initializelook consistent and should be behavior‑preserving for existing callers while exposing the new configuration surface. Nice, minimal wiring change.As a small follow‑up, consider:
- Updating the method doc comment (and/or user‑facing docs) to briefly describe what these lists represent (units, example values, default behavior when
null).- Verifying the parameter names, types, and ordering still exactly match
sdk.Parse.initializeinparse_server_sdk≥9.5.0 so the@overrideremains valid over time.Also applies to: 107-109
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/flutter/lib/parse_server_sdk_flutter.dart(2 hunks)packages/flutter/pubspec.yaml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Agent
- GitHub Check: Test Flutter 3.38, Ubuntu, WASM
- GitHub Check: Test Flutter 3.38, Windows
- GitHub Check: Test Flutter beta
- GitHub Check: Test Flutter 3.38, macOS
- GitHub Check: Test Dart 3.10, Windows
- GitHub Check: Test Flutter 3.38, Ubuntu
🔇 Additional comments (1)
packages/flutter/pubspec.yaml (1)
28-28: Dependency constraint bump correctly aligned with new retry parametersRaising
parse_server_sdkto>=9.5.0 <10.0.0is the right move so the newrestRetryIntervals*named parameters exist onsdk.Parse.initialize. As long as CI passes against 9.5.x and release notes mention this min-version change, this looks good to ship.Please double-check against the
parse_server_sdk9.5.x changelog that there are no other breaking behavior changes for APIs used by this Flutter wrapper.
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/flutter/test/parse_client_configuration_test.dart (1)
22-23: New retry interval fields are correctly exercised in testsPassing
restRetryIntervals/restRetryIntervalsForWritesthroughParse().initializeand asserting them onParseCoreDatagives solid wiring coverage and addresses the previous “missing tests” concern.If you want to go a step further, you could add a second test case that omits these parameters to implicitly assert the SDK’s default behavior, but that’s optional and not a blocker.
Also applies to: 38-39
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/flutter/lib/parse_server_sdk_flutter.dart(3 hunks)packages/flutter/test/parse_client_configuration_test.dart(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Test Flutter 3.38, Ubuntu, WASM
- GitHub Check: Test Flutter 3.38, Windows
- GitHub Check: Test Flutter beta
- GitHub Check: Test Flutter 3.38, macOS
- GitHub Check: Test Flutter 3.38, Ubuntu
- GitHub Check: Test Dart 3.10, Windows
🔇 Additional comments (2)
packages/flutter/lib/parse_server_sdk_flutter.dart (2)
50-59: Retry parameter documentation is clear and developer-friendlyThe added doc comments for
restRetryIntervalsandrestRetryIntervalsForWritesclearly describe scope (read vs write operations), units, defaults, and the idempotency caveat. This is exactly the kind of detail Flutter users need at the wrapper level.
79-80: Retry parameters are correctly threaded through to the base SDK; keep in sync with upstream signatureThe new
restRetryIntervals/restRetryIntervalsForWritesnamed parameters inParse.initializeand their forwarding tosuper.initializelook correct and non-invasive; they preserve existing behavior when leftnulland let the underlying SDK own the defaults.Going forward, it would be good to treat this override as mirroring the base SDK’s
initializesignature and review it whenever the upstream API changes, to avoid subtle drift (e.g., new retry-related fields or changed semantics).Also applies to: 117-118
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/flutter/test/parse_client_configuration_test.dart (1)
42-62: Default-values regression test is solid; minor intent and coupling nits.This test nicely guards default behavior for debug, session handling, and the new retry lists, including ensuring writes default to an empty list. Two small suggestions: (1) the comment “only required parameters” is slightly misleading since
appName,appPackageName, andappVersionare also provided—either adjust the comment or assert those fields as well; and (2) since the expected retry defaults mirror constants from the core Dart SDK, be aware this test will need updating if upstream changes those defaults.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/flutter/test/parse_client_configuration_test.dart(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test Flutter 3.38, Windows
- GitHub Check: Test Flutter 3.38, Ubuntu
- GitHub Check: Test Flutter 3.38, macOS
- GitHub Check: Test Flutter beta
- GitHub Check: Test Flutter 3.38, Ubuntu, WASM
🔇 Additional comments (1)
packages/flutter/test/parse_client_configuration_test.dart (1)
22-23: New retry interval assertions look correct; please confirm alignment with core SDK.Passing
restRetryIntervals/restRetryIntervalsForWriteshere and asserting them viaParseCoreData()is a good, direct verification of the new plumbing from Flutter into the underlying SDK. Please double‑check that these fields map 1:1 (type and semantics, e.g. units and ordering) to the corresponding properties inparse_server_sdkv9.5.0 so the test won’t diverge from upstream behavior on future upgrades.Also applies to: 38-39
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.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Pull Request
Issue
Closes: #1043
Approach
This PR exposes the retry configuration parameters in the Flutter package wrapper to match the functionality already available in the Dart package (v9.5.0).
Changes:
restRetryIntervalsparameter toParse.initialize()in the Flutter packagerestRetryIntervalsForWritesparameter toParse.initialize()in the Flutter packageThis allows Flutter developers to configure automatic retry behavior for REST API requests, including separate retry configurations for read operations (GET, DELETE) versus write operations (POST, PUT).
Tasks
Summary by CodeRabbit
New Features
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.