feat(dart): add TransformationOptions for ingestion transporter configuration#6321
feat(dart): add TransformationOptions for ingestion transporter configuration#6321MarioAlexandruDan wants to merge 22 commits into
Conversation
- Add TransformationOptions to client_core - Add transformationOptions field to ClientOptions - Generate algolia_client_ingestion Dart package - Add ingestion transporter wiring to SearchClient template - Implement chunkedPush, saveObjectsWithTransformation, partialUpdateObjectsWithTransformation, and replaceAllObjectsWithTransformation extensions - Update pubspec_overrides across docs/tests/playground
✔️ Code generated!
📊 Benchmark resultsBenchmarks performed on the method using a mock server, the results might not reflect the real-world performance.
|
Up to standards ✅🟢 Issues
|
b15c29b to
44910b3
Compare
a7ec108 to
ee6ec15
Compare
d1bdb33 to
026e618
Compare
…deduplicate additionalProperties, Platform alias
… handling, and requestOptions timeouts in tests
…ix Mustache traversal
…Exception is RuntimeException
…thout endpoint-level timeouts
Fluf22
left a comment
There was a problem hiding this comment.
I don't understand the changes you made around server timeouts, I might be missing something
| for (var retries = 0; retries < 50; retries++) { | ||
| try { | ||
| await transporter.getEvent( | ||
| runID: runID, | ||
| eventID: eventID, | ||
| requestOptions: requestOptions, | ||
| ); | ||
| return; | ||
| } on AlgoliaApiException catch (e) { | ||
| if (e.statusCode != 404) rethrow; | ||
| } | ||
| await Future<void>.delayed( | ||
| Duration(milliseconds: (retries * 1500).clamp(0, 5000)), | ||
| ); | ||
| } |
There was a problem hiding this comment.
Silently returns after 50 retries
Add a throw exception at the end
| required String eventID, | ||
| RequestOptions? requestOptions, | ||
| }) async { | ||
| for (var retries = 0; retries < 50; retries++) { |
There was a problem hiding this comment.
The default is 100 now, and we want to pass maxRetries from the calling method I believe (check Dart jira ticket for maxRetries update)
| connectTimeout: ingestionOpts?.connectTimeout ?? const Duration(seconds: 25), | ||
| readTimeout: ingestionOpts?.readTimeout ?? const Duration(seconds: 25), | ||
| writeTimeout: ingestionOpts?.writeTimeout ?? const Duration(seconds: 25), |
There was a problem hiding this comment.
Shouldn't we let the ingestion code default the right duration?
Because if we change in the future, we would have two places to change instead of one
| ) + {{/vendorExtensions.x-timeouts}}{{^vendorExtensions.x-timeouts}}RequestOptions( | ||
| writeTimeout: Duration(milliseconds: {{{serverWriteTimeout}}}), | ||
| readTimeout: Duration(milliseconds: {{{serverReadTimeout}}}), | ||
| connectTimeout: Duration(milliseconds: {{{serverConnectTimeout}}}), |
There was a problem hiding this comment.
Not sure to understand why you added that
🧭 What and Why
Part of the effort to standardize transformationOptions across all 11 language SDKs. Depends on the JS + Foundation ticket for CTS infrastructure.
🎟 JIRA Ticket API-379:
Changes included:
🧪 Test