| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
There was a problem hiding this comment.
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.
Sorry, something went wrong.
|
Nice work covering all three providers, and the tests for both retry=True and the default path are solid. One structural ask: DatabaseOptions._endpoint and FirestoreOptions._endpoint skip EventHandlerOptions._endpoint (they call RuntimeOptions._endpoint(self, ...) directly, I assume because the parent does not handle eventFilterPathPatterns) and re-implement the retry=... line inline. That leaves three copies of the retry logic, and any future change to how EventHandlerOptions._endpoint builds the event trigger (new trigger fields, changed defaults) would silently not apply to Database and Firestore. You already moved Storage over to delegating in 23bd29e, please do the same for these two, e.g. by letting the parent accept optional path patterns: # EventHandlerOptions._endpoint
event_trigger = _manifest.EventTrigger(
eventType=kwargs["event_type"],
retry=self.retry if self.retry is not None else False,
eventFilters=kwargs["event_filters"],
eventFilterPathPatterns=kwargs.get("event_filters_path_patterns"),
)then both subclasses build their filters and call super()._endpoint(...). Worth checking the manifest asdict output stays identical for the no-path-patterns providers (a None there may need omitting rather than serialising). If there is a reason the bypass is deliberate that I am missing, happy to hear it. |
Sorry, something went wrong.
…e' of https://github.com/firebase/firebase-functions-python into @invertase/feat-add-retry-for-storage-firestore-database
| Back | FazBrowse Home | New Git URL |
Fixes #165
This PR maintains parity between the Python and JavaScript SDKs by adding retry support for Storage, Firestore, and Realtime Database.