feat: add retry support for firestore, storage and realtime database#298
feat: add retry support for firestore, storage and realtime database#298IzaakGough wants to merge 1 commit into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates StorageOptions, DatabaseOptions, and FirestoreOptions to inherit from EventHandlerOptions instead of RuntimeOptions, allowing the retry configuration to be dynamically set. Corresponding unit tests have been added to verify this behavior. Feedback suggests simplifying the _endpoint method in StorageOptions by leveraging super()._endpoint directly now that it inherits from EventHandlerOptions.
| event_trigger = _manifest.EventTrigger( | ||
| eventType=kwargs["event_type"], | ||
| retry=False, | ||
| retry=self.retry if self.retry is not None else False, | ||
| eventFilters=event_filters, | ||
| ) | ||
|
|
||
| kwargs_merged = { | ||
| **_dataclasses.asdict(super()._endpoint(**kwargs)), | ||
| **_dataclasses.asdict(RuntimeOptions._endpoint(self, **kwargs)), | ||
| "eventTrigger": event_trigger, | ||
| } | ||
| return _manifest.ManifestEndpoint(**_typing.cast(dict, kwargs_merged)) |
There was a problem hiding this comment.
Since StorageOptions now inherits from EventHandlerOptions, you can simplify _endpoint by calling super()._endpoint directly. This avoids manually constructing the EventTrigger and merging dictionaries, leveraging the base class implementation as intended.
return super()._endpoint(**kwargs, event_filters=event_filters)
2 participants
Fixes #165
This PR maintains parity between the Python and JavaScript SDKs by adding retry support for Storage, Firestore, and Realtime Database.